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