----- Original Message ----- > From: "Russell Bryant" <rbry...@redhat.com> > To: andrewbog...@gmail.com > Cc: "Andrew Bogott" <abog...@wikimedia.org>, openstack@lists.launchpad.net > Sent: Monday, July 2, 2012 3:26:56 PM > Subject: Re: [Openstack] best practices for merging common into specific > projects > > On 07/02/2012 03:16 PM, Andrew Bogott wrote: > > Background: > > > > The openstack-common project is subject to a standard > > code-review > > process (and, soon, will also have Jenkins testing gates.) Sadly, > > patches that are merged into openstack-common are essentially > > orphans. > > Bringing those changes into actual use requires yet another step, a > > 'merge from common' patch where the code changes in common are > > copied > > into a specific project (e.g. nova.) > > Merge-from-common patches are generated via an automated > > process. > > Specific projects express dependencies on specific common > > components via > > a config file, e.g. 'nova/openstack-common.conf'. The actual file > > copy > > is performed by 'openstack-common/update.py,' and its behavior is > > governed by the appropriate openstack-common.conf file. > > More background: > > http://wiki.openstack.org/CommonLibrary > > > Questions: > > > > When should changes from common be merged into other projects? > > What should a 'merge-from-common' patch look like? > > What code-review standards should core committers observe when > > reviewing merge-from-common patches? > > > > Proposals: > > > > I. As soon as a patch drops into common, the patch author > > should > > submit merge-from-common patches to all affected projects. > > A. (This should really be done by a bot, but that's not going > > to > > happen overnight) > > All of the APIs in openstack-common right now are considered to be in > incubation, meaning that breaking changes could be made. I don't > think > automated merges are good for anything in incubation. > > Automation would be suitable for stable APIs. Once an API is no > longer > in incubation, we should be looking at how to make releases and treat > it > as a proper library. The copy/paste madness should be limited to > APIs > still in incubation. > > There are multiple APIs close or at the point where I think we should > be > able to commit to them. I'll leave the specifics for a separate > discussion, but I think moving on this front is key to reducing the > pain > we are seeing with code copying. > > > II. In the event that I. is not observed, merge-from-common > > patches > > will contain bits from multiple precursor patches. That is not > > only OK, > > but encouraged. > > A. Keeping projects in sync with common is important! > > B. Asking producers of merge-from-common patches to understand > > the > > full diff will discourage the generation of such merges. > > I don't see this as much different as any other patches to nova (or > whatever project is using common). It should be a proper patch > series. > If the person pulling it in doesn't understand the merge well enough > to > produce the patch series with proper commit messages, then they are > the > wrong person to be doing the merge in the first place.
I went on a bit of a rant about this on IRC yesterday. While I agree a patch series is appropriate for many new features and bug fixes I don't think it should be required for keeping openstack-common in sync. Especially since we don't merge tests from openstack-common which would help verify that the person doing the merges doesn't mess up the order of the patchset. If we were to include the tests from openstack-common in each project this could change my mind. If someone wants to split openstack-common changes into patchsets that might be Okay in small numbers. If you are merging say 5-10 changes from openstack common into all the various openstack projects that could translate into a rather large number of reviews (25+) for things that have been already reviewed once. For me using patchsets to keep openstack-common in sync just causes thrashing of Jenkins, SmokeStack, etc. for things that have already been gated. Seems like an awful waste of review/CI time. In my opinion patchsets are the way to go with getting things into openstack-common... but not when syncing to projects. Hopefully this situation is short lived however and we start using a proper library sooner rather than later. > > > III. Merge-from-common patches should be the product of a single > > unedited run of update.py. > > Disagree, see above. > > > A. If a merge-from-common patch breaks pep8 or a test in nova, > > don't fix the patch; fix the code in common. > > Agreed. > > > IV. Merge-from-common patches are 'presumed justified'. That > > means: > > A. Reviewers of merge-from-common patches should consider test > > failures and pep8 breakages, and obvious functional problems. > > B. Reviewers of merge-from-common patches should not consider > > the > > usefulness, design, etc. of merge-from-common patches. Such > > concerns > > need to be handled within the common review process. > > C. Merges from common should get reviewed early and approved > > readily > > in order to avoid project divergence > > I think core reviewers of projects using openstack-common are > justified > in doing critical review of new code being used by their project. A > core reviewer of nova for examp[le may have a very good reason why > the > code is not suitable for consumption in nova that openstack-common > reviewers didn't think of. > > I don't think blind approvals are ever a good idea. > > > V. Changes to openstack-common.conf are a special case. > > A. Patches with openstack-common.conf changes should include > > the > > relevant merge-from-common changes. > > B. Such patches should /not/ include any other > > merge-from-common > > changes. > > C. Such patches should not only include the common merges, but > > should also include whatever code changes make use of the new > > merge. > > D. Such patches require the same justification and scrutiny as > > any > > other standard project patch. > > This all seems fine to me. > > > Please discuss! If we're able to reach general agreement about > > this > > process, I will document the process in the openstack-common > > HACKING > > guidelines. > > > -- > Russell Bryant > > > > _______________________________________________ > 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