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

Erik Joelsson erikj at openjdk.java.net
Tue Apr 19 15:16:51 UTC 2022


On Mon, 18 Apr 2022 14:44:17 GMT, Guoxiong Li <gli at openjdk.org> wrote:

>> I misunderstood your idea previously. It is better to add two more parameters to the existing handle method than overload it.
>> 
>> After adding two more parameters, the signatures of the method `handle` in all the commands need to be changed, too. Is it the expected behaviour? I need to confirm it, because it may change many files.
>> 
>> ---
>> 
>> The method `CommandHandler#changeLabelsAfterComment`  can be added if we want to control it explicitly.
>> If the `changeLabelsAfterComment` is true, it has these three steps:
>> 1. add/remove labels before comment (in the method `handle`)
>> 2. comment (in the method `processCommand`)
>> 3. add/remove labels after comment (in the method `processCommand`)
>> 
>> If the `changeLabelsAfterComment` is false, it has only two steps:
>> 1. add/remove labels before comment (in the method `handle`)
>> 2. comment (in the method `processCommand`)
>>  
>> If we choose not to add the method `CommandHandler#changeLabelsAfterComment`, we may do the test `labelsToAdd is not empty && labelsToRemove is not empty` to implicitly confirm whether the command want to change the labels after commenting. This implicit way actually make the same three steps : 
>> 1. add/remove labels before comment (in the method `handle`)
>> 2. comment (in the method `processCommand`)
>> 3. add/remove labels after comment if existing (in the method `processCommand`)
>> 
>> What's your opinion, explicitly or implicitly?
>
> Now, I think the implicit way is better and follow your idea above.

I don't think we need `CommandHandler#changeLabelsAfterComment`, it just adds extra complexity.

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

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


More information about the skara-dev mailing list