[Openstack] best practices for merging common into specific projects

Joshua Harlow harlowja at yahoo-inc.com
Mon Jul 2 21:54:27 UTC 2012


Maybe its time to break out of that incubation??

Seems like if most projects are depending on these config files for code copying that that break out has really already occurred, but it just hasn't been written down or made official?

I don't quite understand how a project can be in incubation, but has components that are required for other projects to work?

Isn't this the same as having library versions where a project would pull in X.Y.Z openstack-common library version (with the features it wants) while the openstack-common people work on X.Y.Z + 1?

Then when X.Y.Z + 1 is approved 'stable' they can move to X.Y.Z+1.

On 7/2/12 12:26 PM, "Russell Bryant" <rbryant at redhat.com> wrote:

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.

> 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 at lists.launchpad.net
Unsubscribe : https://launchpad.net/~openstack
More help   : https://help.launchpad.net/ListHelp

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack/attachments/20120702/0bfd09da/attachment.html>


More information about the Openstack mailing list