Open Stack

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 :)

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 <rbtcollins at hp.com>
Distinguished Technologist
HP Converged Cloud