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

Erik Joelsson erikj at openjdk.java.net
Wed Dec 15 22:11:54 UTC 2021

On Wed, 15 Dec 2021 12:44:11 GMT, Kevin Rushforth <kcr 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?

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.


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

More information about the skara-dev mailing list