RFR: 1071: auto-set `/csr` flag from JBS state
Erik Joelsson
erikj at openjdk.java.net
Wed Nov 24 17:10:45 UTC 2021
On Wed, 24 Nov 2021 15:57:55 GMT, Guoxiong Li <gli at openjdk.org> wrote:
> Hi all,
>
> Currently, the author of the PR or a reviewer could type `/csr` in the comment to direct the need of the CSR.
> It is good for the bot to automatically scan the `scr for` link and set the `csr` label for the PR.
>
> This patch adds the automatically scanning feature. And the corresponding tests are added.
>
> Thanks for taking the time to review.
>
> Best Regards,
> -- Guoxiong
bots/csr/src/main/java/org/openjdk/skara/bots/csr/CSRBot.java line 95:
> 93: var resolution = csr.properties().get("resolution");
> 94: if (resolution == null || resolution.isNull()) {
> 95: log.info("CSR issue resolution is null for " + describe(pr) + ", not removing CSR label");
I think the log message should reflect if we are leaving the label or adding it.
bots/csr/src/test/java/org/openjdk/skara/bots/csr/CSRBotTests.java line 168:
> 166: TestBotRunner.runPeriodicItems(bot);
> 167:
> 168: // The bot should have kept the CSR label
Is this a check for if the bot added the label automatically?
bots/csr/src/test/java/org/openjdk/skara/bots/csr/CSRBotTests.java line 172:
> 170:
> 171: // Add CSR label
> 172: pr.addLabel("csr");
Adding the label here has no effect.
bots/pr/src/main/java/org/openjdk/skara/bots/pr/CSRCommand.java line 81:
> 79:
> 80: // Change the CSR link relationship from `csr for` to `relates to`
> 81: // so that the CSRBot won't add the `csr` label automatically.
I'm not sure this is a good idea. The only reason to keep the /csr command around would be to have a way of manually correcting the state of a PR if the bots somehow fail, but I'm leaning towards just removing the command. If a CSR is already linked in JBS, but is later deemed to not be needed, I think the user should fix that in JBS rather than through a Skara command.
I think this needs some discussion and input from more people.
-------------
PR: https://git.openjdk.java.net/skara/pull/1245
More information about the skara-dev
mailing list