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