<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Sep 8, 2014 at 1:14 PM, Robert Collins <span dir="ltr"><<a href="mailto:robertc@robertcollins.net" target="_blank">robertc@robertcollins.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I hope the subject got your attention :).<br>
<br>
This might be a side effect of my having too many cosmic rays, but its<br>
been percolating for a bit.<br>
<br>
tl;dr I think we should drop our 'needs 2x+2 to land' rule and instead<br>
use 'needs 1x+2'. We can ease up a large chunk of pressure on our<br>
review bottleneck, with the only significant negative being that core<br>
reviewers may see less of the code going into the system - but they<br>
can always read more to stay in shape if thats an issue :)<br>
<br></blockquote><div><br></div><div>And you can always use +1, if you feel that another core should review.<br></div><br><div>-Angus<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Thats it really - below I've some arguments to support this suggestion.<br>
<br>
-Rob<br>
<br>
# Costs of the current system<br>
<br>
Perfectly good code that has been +2'd sits waiting for a second +2.<br>
This is a common complaint from folk suffering from review latency.<br>
<br>
Reviewers spend time reviewing code that has already been reviewed,<br>
rather than reviewing code that hasn't been reviewed.<br>
<br>
# Benefits of the current system<br>
<br>
I don't think we gain a lot from the second +2 today. There are lots<br>
of things we might get from it:<br>
<br>
- we keep -core in sync with each other<br>
- better mentoring of non-core<br>
- we avoid collaboration between bad actors<br>
- we ensure -core see a large chunk of the changes going through the system<br>
- we catch more issues on the code going through by having more eyeballs<br>
<br>
I don't think any of these are necessarily false, but equally I don't<br>
they are necessarily true.<br>
<br>
## keeping core in sync<br>
<br>
For a team of (say) 8 cores, if 2 see each others comments on a<br>
review, a minimum of 7 reviews are needed for a reviewer R's thoughts<br>
on something to be disseminated across the team via osmosis. Since<br>
such thoughts probably don't turn up on every review, the reality is<br>
that it may take many more reviews than that: it is a thing, but its<br>
not very effective vs direct discussion.<br>
<br>
## mentoring of non-core<br>
<br>
This really is the same as the keeping core in sync debate, except<br>
we're assuming that the person learning has nothing in common to start<br>
with.<br>
<br>
## avoiding collaboration between bad actors<br>
<br>
The two core requirement means that it takes three people (proposer +<br>
2 core) to collaborate on landing something inappropriate (whether its<br>
half baked, a misfeature, whatever).  Thats only 50% harder than 2<br>
people (proposer + 1 core) and its still not really a high bar to<br>
meet. Further, we can revert things.<br>
<br>
## Seeing a high % of changes<br>
<br>
Consider nova - <a href="http://russellbryant.net/openstack-stats/nova-reviewers-90.txt" target="_blank">http://russellbryant.net/openstack-stats/nova-reviewers-90.txt</a><br>
Core team size: 21 (avg 3.8 reviews/day) [79.8/day for the whole team]<br>
Changes merged in the last 90 days: 1139 (12.7/day)<br>
<br>
Each reviewer can only be seeing 30% (3.8/12.7) of the changes to nova<br>
on average (to keep up with 12/day landing). So they're seeing a lot,<br>
but there's more that they aren't seeing already. Dropping 30% to 15%<br>
might be significant. OTOH seeing 30% is probably not enough to keep<br>
up with everything on its own anyway - reviewers are going to be<br>
hitting new code regularly.<br>
<br>
## Catching more issues through more eyeballs<br>
<br>
I'm absolutely sure we do catch more issues through more eyeballs -<br>
but what eyeballs look at any given review is pretty arbitrary. We<br>
have a 30% chance of any given core seeing a given review (with the<br>
minimum of 2 +2s). I don't see us making a substantial difference to<br>
the quality of the code that lands via the second +2 review. I observe<br>
that our big themes on quality are around systematic changes in design<br>
and architecture, not so much the detail of each change being made.<br>
<br>
<br>
-Rob<br>
<span class="HOEnZb"><font color="#888888"><br>
<br>
--<br>
Robert Collins <<a href="mailto:rbtcollins@hp.com">rbtcollins@hp.com</a>><br>
Distinguished Technologist<br>
HP Converged Cloud<br>
<br>
_______________________________________________<br>
OpenStack-dev mailing list<br>
<a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</font></span></blockquote></div><br></div></div>