openstack team mailing list archive
Mailing list archive
Re: Merge proposals and criteria for approval
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
* 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?
>> -----Original Message-----
>> From: openstack-bounces+armando.migliaccio=eu.citrix.com@xxxxxxxxxxxxxxxxxxx
>> bounces+armando.migliaccio=eu.citrix.com@xxxxxxxxxxxxxxxxxxx] On Behalf Of Jay
>> Sent: 21 December 2010 17:48
>> To: Rick Clark
>> Cc: openstack@xxxxxxxxxxxxxxxxxxx
>> 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@xxxxxxxxxxxxx> 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.
>>> Mailing list: https://launchpad.net/~openstack
>>> Post to : openstack@xxxxxxxxxxxxxxxxxxx
>>> Unsubscribe : https://launchpad.net/~openstack
>>> More help : https://help.launchpad.net/ListHelp
>> Mailing list: https://launchpad.net/~openstack
>> Post to : openstack@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~openstack
>> More help : https://help.launchpad.net/ListHelp
> Mailing list: https://launchpad.net/~openstack
> Post to : openstack@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~openstack
> More help : https://help.launchpad.net/ListHelp
Description: OpenPGP digital signature