FWIW, I agree with all your points on this, Rick. On Tue, Dec 21, 2010 at 11:43 AM, Rick Clark <r...@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@lists.launchpad.net > Unsubscribe : https://launchpad.net/~openstack > More help : https://help.launchpad.net/ListHelp > >
_______________________________________________ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
