Open Stack

On 30 April 2013 09:50, Russell Bryant <rbryant at redhat.com> wrote:
> On 04/29/2013 05:04 PM, Robert Collins wrote:

  * No more big landings [except the purely mechanical]. Set a hard
>> limit - maybe 500 lines of diff. Big landings are more risky per line
>> of diff than small ones due to reviewer cognitive overhead - reviewers
>> get non-linearly less effective the larger the review.
>>
>
> I don't think we can set a # of lines that always makes sense.  However,
> I feel like in Nova we already do a nice job of pushing back hard on
> large patches in favor of breaking them up into a reasonable patch series.
>
> https://wiki.openstack.org/wiki/GitCommitMessages
>
> So, at least for Nova, this is business as usual.

In principle yes. However I think we need to give more specific
guidance to contributors... This is about setting expectations for
submitters as much as it is about reviewers: the documentation link
you gave already permits reviewers to say 'this patch does more than
one conceptual thing, please split it'; but when was the last time you
saw a non-mechanical 500 line patch that did *one thing* ?


>
> Most of this seems good.  It seems as if there's tension between:
>
> 1) Ensuring utmost quality so you can deploy any revision
>
> and
>
> 2) Being less careful with some new features and justifying it by having
> them disabled by default.
>
> I don't see the value in this disabled-by-default thing.  Another one of
> your points stresses the fact that if something's not ready, it's not
> ready, which I definitely agree with.

I suspect this is going to be a drawn out conversation, but the CD
benefit from it is risk avoidance. For two reasons:
 - Landing a 50 patch change series all at once after everyone agrees
it is ready is much more risk than landing it as each patch is
reviewed: the total delta per day is more manageable.
 - There are often questions about quality that the code itself cannot
answer (such as performance at scale, user comprehension,
interoperability with proprietary hardware) which the review mechanism
isn't aimed at answering: making features off by default prevents
those risks affecting everyone, instead they only impact early
adopters who have volunteered for them.

-Rob

--
Robert Collins <rbtcollins at hp.com>
Distinguished Technologist
HP Cloud Services