reviewer attribution

[David Holmes]

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
> work.
> 
> 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 
suggests.

Cheers,
David
-----

> 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:
> 
> https://github.com/openjdk/jdk/pull/1003
> 
> 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".
> 
> Roland.
>