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.
--
Antoine "hashar" Musso