reviewer attribution

[Kevin Rushforth]

My experience with this on JavaFX is that once people get used to the 
idea of doing the review on GitHub, it isn't really a problem. I view 
this as (mostly) a step forward not a step backwards, since the review 
approval is explicit and you don't need to guess whether a reviewer is 
satisfied that all their concerns have been addressed.

Having said that, I do like the idea of being able to manually add a 
"non-counting" reviewer manually, which is useful for those interacting 
with the mailing list. That seems to be working well. However, once a 
reviewer has done a review on GitHub, and has requested changes, I think 
it would be wrong to override that.

-- Kevin


On 11/17/2020 12:58 AM, 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.
>
> 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.
>