Open Stack

On 09/08/2014 08:52 AM, Russell Bryant wrote:
> On 09/08/2014 05:17 AM, Steven Hardy wrote:
>> On Mon, Sep 08, 2014 at 03:14:24PM +1200, Robert Collins 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 :)
>>
>> I think this may be a sensible move, but only if it's used primarily to
>> land the less complex/risky patches more quickly.
>>
>> As has been mentioned already by Angus, +1 can (and IMO should) be used for
>> any less trival and/or risky patches, as the more-eyeballs thing is really
>> important for big or complex patches (we are all fallible, and -core folks
>> quite regularly either disagree, spot different types of issue, or just
>> have better familiarity with some parts of the codebase than others).
>>
>> FWIW, every single week in the Heat queue, disagreements between -core
>> reviewers result in issues getting fixed before merge, which would result
>> in more bugs if the 1x+2 scheme was used unconditionally.  I'm sure other
>> projects are the same, but I guess this risk can be mitigated with reviewer
>> +1 discretion.
> 
> Agreed with this.  I think this is a worthwhile move for simpler
> patches.  I've already done it plenty of times for a very small category
> of things (like translations updates).  It would be worth having someone
> write up a proposal that reflects this, with some examples that
> demonstrate patches that really need the second review vs others that
> don't.  In the end, it has to be based on trust in a -core team member
> judgement call.

One of the review queries I've got is the existing Nova patches which
already have 1 +2 on those. I'd say ~ 25% of them I find an issue in.
I'm assuming another 25% of them had an issue I didn't find.

2 +2 has been part of OpenStack culture for a long time, and there is a
good reason for it, it really does keep bugs out.

It should also be clear that the subject of this email really should
have been "merging code faster", because nothing in here doubles the
review bandwidth, it just provides us with less review coverage.

I'm currently less convinced that raw core merge speed is our primary
issue right now. I think it's a symptom of accumulated debt, and
complexity growth from the features over the last couple of cycles.
Treating symptoms means we'll just be discussing this in another 6
months (if we're lucky) in a new process change thread.

	-Sean

-- 
Sean Dague
http://dague.net