kevin.rushforth at oracle.com
Tue Nov 17 13:01:09 UTC 2020
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.
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
> 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:
> 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