RFR: 1071: auto-set `/csr` flag from JBS state

Erik Joelsson erikj at openjdk.java.net
Wed Nov 24 17:21:02 UTC 2021


On Wed, 24 Nov 2021 17:07:47 GMT, Erik Joelsson <erikj 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/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.

I read up more on the history here. The /csr command can be invoked prior to creating a CSR issue in JBS, so it still makes sense to have the command. Given this, then invoking "/csr unneeded" can happen in two different scenarios:

1. No CSR has been filed yet, then just remove the csr label
2. A CSR is linked in JBS, in this case the CSRBot will likely automatically add the label again

I still don't think it makes sense to have this command go and change things in JBS. In case 2, a better cause of action would be to tell the user in an error reply that a CSR already exists and needs to be closed/removed first.

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

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


More information about the skara-dev mailing list