RFR: 1288: Adjust the implementation of the 'reviewers' command to match the documentation

Guoxiong Li gli at openjdk.java.net
Tue Mar 29 13:15:58 UTC 2022


On Wed, 15 Dec 2021 22:09:19 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> Unless there is a corner case that I haven't hit, the implementation is working as expected now. I will need to look at your proposed change more closely, but I don't see the need to change the existing logic.
>> 
>> What problem are you trying to solve?
>
>> Unless there is a corner case that I haven't hit, the implementation is working as expected now. I will need to look at your proposed change more closely, but I don't see the need to change the existing logic.
>> 
>> What problem are you trying to solve?
> 
> In the linked PR, they tried to run `/reviewers 2 reviewer`, which gave a very weird error. At least that part is completely wrong in the current implementation. There isn't really any test coverage for using the command with a role argument. As for the rest, I'm still investigating. 
> 
> The command as implemented does not do what the documentation at https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullRequestCommands-/reviewers says it's supposed to do, that I definitely agree on. I think one point of confusion is if multiple command invocations should be added together, or just override the previous one, and should the override apply globally or just to the specified role? 
> 
> Before we go any further with an implementation, I think we need to agree on what the desired behavior actually is. The bug comments seems like a good forum for suggesting and commenting on bot behavior.

@erikj79 Could I get your help to sponsor this patch? Thanks a lot.

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

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


More information about the skara-dev mailing list