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

Erik Joelsson erikj at openjdk.java.net
Mon Apr 11 13:30:24 UTC 2022


On Sat, 9 Apr 2022 06:43:11 GMT, Guoxiong Li <gli at openjdk.org> wrote:

>> bots/pr/src/main/java/org/openjdk/skara/bots/pr/JEPCommand.java line 99:
>> 
>>> 97:                 reply.println("only [Reviewers](https://openjdk.java.net/bylaws#reviewer) can determine that a JEP request is not needed.");
>>> 98:                 return;
>>> 99:             }
>> 
>> I don't think I agree with this check. If the author is able to link the PR to a JEP, then the author should be able to correct a mistakenly linked JEP.
>
> The author can use the `/jep <jep-id>|<issue-id>` again if the author linked the wrong JEP. This check follows the convention of the `csr` command. It can protect the situation that the author uses the command `/jep unneeded` and integrates the patch unexpectly.
> 
> Actually, the `/jep unneeded` command is rarely used and the author can request the reviewers to do that if the patch really don't need a JEP to be targeted. Preventing the unexpected (and maybe wrong) code from integrating should be the most important thing we need to ensure even though it has not happened in the past.

I understand that this is how it would work with the current logic, and I don't agree with it. Declaring the need for a JEP and a CSR are different, so they do not need to follow the same model (though I'm not sure I agree with the limitation on `/csr unneeded` either).

In order for a committer to mishandle this, they would have to actively issue `/jep unneeded` and then integrate. I think it's unlikely to happen by mistake, and mistakes are what this feature intends to protect against. We already trust committers to handle `/integrate` on their own, this isn't about protecting against malicious committers.

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

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


More information about the skara-dev mailing list