Open Stack

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.

-- 
Russell Bryant