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