[Openstack] Merge proposals and criteria for approval

Rick Clark rick at openstack.org
Tue Dec 21 18:55:04 UTC 2010


That is an excellent question.  I'm not sure we have any criteria for
the order that proposals are done.  I think we can come up with some
recommendations.

* Oldest first.
* If we have a dependency chain we should start at the top dependency
* Developers new to the project should start with shorter simpler merges
* big complex files patch sets that touch lots of should be merged as
early as possible, since they are likely to break subsequent merge requests

Any others you can think of?


On 12/21/2010 12:28 PM, Armando Migliaccio wrote:
> More than thoughts, I have a question: what criteria are selected to order the merges under review? Length of the diff/priority of the blueprint etc?
> 
> Thanks,
> Armando
> 
>> -----Original Message-----
>> From: openstack-bounces+armando.migliaccio=eu.citrix.com at lists.launchpad.net
>> [mailto:openstack-
>> bounces+armando.migliaccio=eu.citrix.com at lists.launchpad.net] On Behalf Of Jay
>> Pipes
>> Sent: 21 December 2010 17:48
>> To: Rick Clark
>> Cc: openstack at lists.launchpad.net
>> Subject: Re: [Openstack] Merge proposals and criteria for approval
>>
>> FWIW, I agree with all your points on this, Rick.
>>
>> On Tue, Dec 21, 2010 at 11:43 AM, Rick Clark <rick at openstack.org> wrote:
>>> As most of you know, the Nova merge queue is starting to back up.  In an
>>> effort to help clear the log jam, I would like to propose some criteria
>>> for how we do code reviews and what we approve.
>>>
>>> Statement of Fact: All code has bugs. Bugs are normal and can have no
>>> impact, or can be critical.  We have all types in our code
>>>
>>> Statement of Fact: Regressions, bugs that break previously working
>>> features, are bad, and should be avoided.
>>>
>>> I think that we should focus on finding regressions at review time not bugs.
>>>
>>> I propose that the following criteria be used for approving code reviews:
>>>  * Architectural soundness
>>>  * regression free
>>>  * Code cleanliness (Pep8 compliance) and style
>>>  * Complete test coverage
>>>  * Documentation
>>>
>>>
>>> Any obvious non-regressing bugs should be filed in Launchpad at review
>>> time by the reviewer.
>>>
>>> Basically I think this will solve a couple problems.  We spend quite a
>>> bit of time in an iterative process with a patches author fixing issues
>>> in a patch.  Some of these need to be fixed by the original author prior
>>> to merge, but many do not.  As soon as a patch is merged it is owned by
>>> everyone and anyone can fix the issues.  This also will allow new
>>> developers to grab relatively simple bugs from merges to cut their teeth
>>> on the code.
>>>
>>> I would also say that breaking a test is not a regression it is a bug.
>>> If a merge breaks a test, but does not break the  actual feature it is a
>>> bug in the test.  But I think this is more controversial, so I'm just
>>> bringing it up for discussion and don't propose any changes.
>>>
>>>
>>> Thoughts?
>>>
>>> Rick
>>>
>>>
>>> _______________________________________________
>>> Mailing list: https://launchpad.net/~openstack
>>> Post to     : openstack at lists.launchpad.net
>>> Unsubscribe : https://launchpad.net/~openstack
>>> More help   : https://help.launchpad.net/ListHelp
>>>
>>>
>>
>> _______________________________________________
>> Mailing list: https://launchpad.net/~openstack
>> Post to     : openstack at lists.launchpad.net
>> Unsubscribe : https://launchpad.net/~openstack
>> More help   : https://help.launchpad.net/ListHelp
> _______________________________________________
> Mailing list: https://launchpad.net/~openstack
> Post to     : openstack at lists.launchpad.net
> Unsubscribe : https://launchpad.net/~openstack
> More help   : https://help.launchpad.net/ListHelp


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 262 bytes
Desc: OpenPGP digital signature
URL: <http://lists.openstack.org/pipermail/openstack/attachments/20101221/a14168d4/attachment.sig>


More information about the Openstack mailing list