openstack team mailing list archive
-
openstack team
-
Mailing list archive
-
Message #00148
Re: Merge proposals and criteria for approval
How about bug fixes? Do they get priority?
How about responsiveness to reviewers' feedback? Would a more responsive contributor get priority over a less responsive one?
Number of branches per contributor?
Just thinking out loud...
> -----Original Message-----
> From: Rick Clark [mailto:rick@xxxxxxxxxxxxx]
> Sent: 21 December 2010 18:55
> To: Armando Migliaccio
> Cc: openstack@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Openstack] 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
> 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@xxxxxxxxxxxxxxxxxx
> >> t
> >> [mailto:openstack-
> >> bounces+armando.migliaccio=eu.citrix.com@xxxxxxxxxxxxxxxxxxx] On
> >> bounces+Behalf Of Jay
> >> Pipes
> >> 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.
> >>>
> >>>
> >>> Thoughts?
> >>>
> >>> Rick
> >>>
> >>>
> >>> _______________________________________________
> >>> 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
>
References