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

[Guoxiong Li]

On Mon, 18 Apr 2022 13:25:26 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> I updated the code just now. The new code changes the PR labels after commenting just as we discussed before.
>> 
>> And I refactored the method `processCommand` to avoid the multi-level nested conditional statements.
>
> This wasn't quite how I envisioned it. I don't think we need to overload the handle method. I also don't think we should use the return value for this. We already use the reply parameter as a kind of return value, a parameter that you modify to return a new state to the caller. My idea was to just add two more parameters to the existing handle method:
> 
> ..., List<String> labelsToAdd, List<String> labelsToRemove)
> 
> The logic in `processCommand` wouldn't need to change much. It would need to instantiate two ArrayLists in the case of `isCommit==false`, and then loop through each list to process the label changes on the pr. The `pr.addComment(writer.toString())` line would need to be moved from the end to just after each of the two `handle` calls.

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?

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

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