RFR: SKARA-1096: New command and label for JEPs, similar to CSR [v10]

[Guoxiong Li]

On Wed, 20 Apr 2022 17:19:45 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> Guoxiong Li has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Polish
>>  - Overload the handle method and revert the change of unrelated commands
>
> bots/jep/src/main/java/org/openjdk/skara/bots/jep/JEPBot.java line 75:
> 
>> 73:                     pr.removeLabel(JEP_LABEL);
>> 74:                 }
>> 75:                 log.info("No jep command found in comment for PR-" + pr.id());
> 
> This is going to log for most PRs every time this WorkItem runs. I would like to lower it to `fine` level. Also, please see CSR bot for a good way of logging pr name/IDs. I would recommend copying the describe method here.

Fixed.

> bots/jep/src/main/java/org/openjdk/skara/bots/jep/JEPBot.java line 82:
> 
>> 80:             if ("unneeded".equals(issueId)) {
>> 81:                 if (pr.labelNames().contains(JEP_LABEL)) {
>> 82:                     pr.removeLabel(JEP_LABEL);
> 
> Please log that action was taken, see example below.

Fixed.

> bots/jep/src/main/java/org/openjdk/skara/bots/jep/JEPBot.java line 84:
> 
>> 82:                     pr.removeLabel(JEP_LABEL);
>> 83:                 }
>> 84:                 log.info("Found `/jep unneeded` command");
> 
> Please include the `describe(pr)` in the log message.

Fixed.

> bots/jep/src/main/java/org/openjdk/skara/bots/jep/JEPBot.java line 116:
> 
>> 114:                 pr.removeLabel(JEP_LABEL);
>> 115:             } else if (!hasTargeted && !pr.labelNames().contains(JEP_LABEL)) {
>> 116:                 pr.addLabel(JEP_LABEL);
> 
> Here we are taking action, I would like this logged at info level. Something like this:
> Suggestion:
> 
>                 log.info("JEP issue " + issue.id() + " found in state " + issueStatus + ", removing JEP label from " + describe(pr);
>                 pr.removeLabel(JEP_LABEL);
>             } else if (!hasTargeted && !pr.labelNames().contains(JEP_LABEL)) {
>                 log.info("JEP issue " + issue.id() + " found in state " + issueStatus + ", adding JEP label to " + describe(pr);
>                 pr.addLabel(JEP_LABEL);

Fixed.

> bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestCommandWorkItem.java line 154:
> 
>> 152:             for (var label : labelsToAdd) {
>> 153:                 if (!pr.labelNames().contains(label)) {
>> 154:                     pr.addLabel(label);
> 
> Could you add an info log about adding and removing labels to the PR.

Fixed.

-------------

PR: https://git.openjdk.java.net/skara/pull/1297