Le 16/03/2016 20:49, Daniel Kinzler a écrit :
There will be an RFC meeting tonight about this: https://phabricator.wikimedia.org/E148
One thing that I don't remember coming up during the summit is:
Can we perhaps get all the people with +1 rights to use them and actually review stuff? So that people with +2 rights can look at things that already have a +1, and can thus avoid getting swamped with lots of patches with lots of low grade issues?
Where would this idea fit into the zoo of tickets we have on the subject?
Hello,
I once proposed to get an additional tier in our code review system, a level that would approve what has been reviewed. I can not find the link though, but I believe I got the inspiration from OpenStack. Their workflow is:
Anyone can Code-Review +1 Core reviewers do Code-Review +2 Maintainers Approve +1 Release manager deal with branches and accepting hotfixes.
They have a convention or a Gerrit enforcement that a changes need two CR+2 before it get approved. Once Approved +1 , the change get merged.
What does it mean for a maintainer? They can craft a query to look for changes that have received a few CR+2, focus on the release quality and almost blindly approve changes in bulk. I am part of such group and it makes it very easy to do the final review and land patches.
Drawback, as a dev you need to seek +2 from a couple core reviewers...
That got shoot down as not being the wiki way. Instead we are much more liberal in giving CR+2 right on the repo, in turn trusting people with such privilege to not CR+2 patches on code they are unfamiliar with. I think it is a good compromise.
If we had some kind of dashboards to list patches that are ready enough, got a fair amount of CR votes, that would probably make life easier for people landing changes. I myself have bookmarked a couple Gerrit queries:
A) for changes on which I am a reviewer and haven't voted yet:
is:open reviewer:self NOT owner:self label:code-review=0,self
That is the first page I hit every morning which list reviews I need to do for others.
B) Changes reviewed with no CR-1/-2 and no verified -1 (Jenkins fail)
is:open reviewer:self NOT owner:self label:code-review=0,self label:code-review=1 NOT label:code-review=-1 NOT label:code-review=-2 NOT label:verified<=0
That is more or less changes I can CR+2 right away.