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
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Mailing list: https://launchpad.net/~openstack Post to : openstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~openstack More help : https://help.launchpad.net/ListHelp
