kevin.rushforth at oracle.com
Wed Nov 18 13:27:18 UTC 2020
Manually adding a reviewer who has made comments, but not formally
approved via the UI, seems a useful feature. I don't think tht manually
adding a reviewer who has done a review requesting changes, which is the
way you indicate that your comments are blocking, is such a good idea.
It seems to me that the drawbacks outweigh any benefit.
On 11/17/2020 6:47 PM, David Holmes wrote:
> On 17/11/2020 6:58 pm, Roland Westrelin wrote:
>> Hi Skara Developers,
>> My experience with Skara has so far been painless. Thanks for the great
>> I noticed one of my PRs has 3 reviewers listed on the right hand size
>> under "reviewers" but a single one listed in the comments. Only that one
>> reviewer has approved the change with the github web interface. Change
>> is cleared for integration so I could push it without proper reviewer
>> attribution. I tried the /reviewer command to force the system to record
>> the 2 other reviewers but that doesn't help.
> I agree that this is a problem. There is a case to be made that
> reviewers just have to get used to going back to the PR and clicking
> "Approve", and that developers have to wait until the reviewers have
> done so. But that is contrary to our position that we don't block
> integrations until the approvals are in place, to allow for trivial
> final updates.
> It is common (for me at least) to do a review and select "comment" or
> "changes requested" for minor issues. They then get fixed, but I don't
> get listed as a Reviewer unless I go back and rubber stamp them. I've
> started doing that but it is a bit of a pain.
> On the flip side someone who just makes a passing comment shouldn't
> automatically get the credit, or responsibility, of being a reviewer.
> I don't see a solution other than allowing a manual override as Roland
>> To me, this feels like a step backward from what we had with mercurial
>> where we were in full control of the reviewer list. There's a risk that
>> even careful developers could push a change without proper reviewer
>> attribution. Also, in the openjdk project, we largely trust developers
>> for doing the right thing. In that spirit, allowing the /reviewer
>> command full power on the reviewer list would make sense for the rare
>> case where it's obvious the change is good to go but someone hasn't
>> explicitly approved it. I feel it's better than having to chase all
>> reviewers to have them click a button or integrating a change without
>> all involved reviewers credited.
>> The PR is:
>> but I emailed reviewers that were not credited to ask them to approve
>> the change so the reviewer list is in the process of being "fixed".
More information about the skara-dev