Open Stack

On 09/08/2014 02:52 PM, 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.
> 

What about simply leaving it as-is and allow people to ninja-approve
things when they feel like it? When in doubt, reviewers should just
stick to the 2x+2 and don't approve the patch.

I'm basically saying the same thing that has been proposed but based on
the current default + an exception to the rule. What changes is the way
people approach reviews. Leaving 2x+2 as the default but allowing
exceptions where people can 1x+2A is better than making the default 1x+2
and asking people to not approve patches if they think it requires more
reviews.

Just my $0.02, I agree with the proposal of trusting cores and letting
them do 1x+2A when they think it's worth it.

Flavio

-- 
@flaper87
Flavio Percoco