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

[Erik Joelsson]

On Tue, 19 Apr 2022 15:45:13 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CommandHandler.java line 38:
>> 
>>> 36:     default void handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInstance, Path scratchPath, CommandInvocation command,
>>> 37:                         List<Comment> allComments, PrintWriter reply, List<String> labelsToAdd, List<String> labelsToRemove) {
>>> 38:     }
>> 
>> I keep coming back to this in my reviewing. It would be better if we did not have to update all those other places where handle() is implemented. 
>> 
>> I might be thinking wrong here, but I believe that you should be able to keep the original `handle(PullRequestBot bot, PullRequest pr, CensusInstance censusInstance, Path scratchPath, CommandInvocation command, List<Comment> allComments, PrintWriter reply)` method, and instead add the new `handle` (with `List<String> labelsToAdd, List<String> labelsToRemove` as a default method, with the implementatation
>> 
>> handle(bot, pr, censusInstance, scratchPath, command, allComments, reply);
>> 
>> 
>> This way, the commands that need the new label handling mechanism can implement the new handle override with the extended argument list. All other commands needs no change.
>> 
>> Once again, I don't feel 100% confident I'm thinking correctly here, but it seems to me like it would work, and be a better solution.
>
> You are correct Magnus, that would be nicer indeed.

The drawback of doing that is that JEPCommand would still need to implement the old handle method, which doesn't make sense for it to do. Unless we also add a default empty implementation for the original handle method.

@lgxbslgx If you want to change it to avoid touching all the other commands, using default methods in the interface instead, that's ok, but I'm also ok leaving it as it is.

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

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