reviewer attribution

Tobias Hartmann tobias.hartmann at oracle.com
Wed Nov 18 06:28:53 UTC 2020


+1 on trusting our committers and allowing a manual overwrite. The bot could issue a warning /
notification to the corresponding reviewer who could then still object.

Best regards,
Tobias

On 18.11.20 03:47, 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
>> 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.
>>


More information about the skara-dev mailing list