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

[Erik Joelsson]

On Tue, 19 Apr 2022 15:40:51 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:

>> Guoxiong Li has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Redo: Change labels after comment.
>>  - Revert: Change labels after comment.
>
> 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.

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

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