<HTML>
<HEAD>
<TITLE>Re: [Openstack] best practices for merging common into specific projects</TITLE>
</HEAD>
<BODY>
<FONT FACE="Calibri, Verdana, Helvetica, Arial"><SPAN STYLE='font-size:11pt'>What about using openstack-common as a library instead of a preprocessor ‘inclusion’ system/copy code around system....??<BR>
<BR>
Maybe its time for that to happen? <BR>
<BR>
It always seemed sort of silly to me that files are being copied around to different projects like this, instead of referring to code in a common library package.<BR>
<BR>
On 7/2/12 12:16 PM, "Andrew Bogott" <<a href="abogott@wikimedia.org">abogott@wikimedia.org</a>> wrote:<BR>
<BR>
</SPAN></FONT><BLOCKQUOTE><FONT FACE="Calibri, Verdana, Helvetica, Arial"><SPAN STYLE='font-size:11pt'> Having spent some time last week writing code for openstack-common,<BR>
and having spent yet more time trying to get those changes into Nova, I<BR>
think it would be useful to define some best practices when crossing the<BR>
boundary between common and other openstack projects.<BR>
<BR>
Background:<BR>
<BR>
The openstack-common project is subject to a standard code-review<BR>
process (and, soon, will also have Jenkins testing gates.) Sadly,<BR>
patches that are merged into openstack-common are essentially orphans. <BR>
Bringing those changes into actual use requires yet another step, a<BR>
'merge from common' patch where the code changes in common are copied<BR>
into a specific project (e.g. nova.)<BR>
Merge-from-common patches are generated via an automated process. <BR>
Specific projects express dependencies on specific common components via<BR>
a config file, e.g. 'nova/openstack-common.conf'. The actual file copy<BR>
is performed by 'openstack-common/update.py,' and its behavior is<BR>
governed by the appropriate openstack-common.conf file.<BR>
<BR>
Questions:<BR>
<BR>
When should changes from common be merged into other projects?<BR>
What should a 'merge-from-common' patch look like?<BR>
What code-review standards should core committers observe when<BR>
reviewing merge-from-common patches?<BR>
<BR>
Proposals:<BR>
<BR>
I. As soon as a patch drops into common, the patch author should<BR>
submit merge-from-common patches to all affected projects.<BR>
A. (This should really be done by a bot, but that's not going to<BR>
happen overnight)<BR>
<BR>
II. In the event that I. is not observed, merge-from-common patches<BR>
will contain bits from multiple precursor patches. That is not only OK,<BR>
but encouraged.<BR>
A. Keeping projects in sync with common is important!<BR>
B. Asking producers of merge-from-common patches to understand the<BR>
full diff will discourage the generation of such merges.<BR>
<BR>
III. Merge-from-common patches should be the product of a single<BR>
unedited run of update.py.<BR>
A. If a merge-from-common patch breaks pep8 or a test in nova,<BR>
don't fix the patch; fix the code in common.<BR>
<BR>
IV. Merge-from-common patches are 'presumed justified'. That means:<BR>
A. Reviewers of merge-from-common patches should consider test<BR>
failures and pep8 breakages, and obvious functional problems.<BR>
B. Reviewers of merge-from-common patches should not consider the<BR>
usefulness, design, etc. of merge-from-common patches. Such concerns<BR>
need to be handled within the common review process.<BR>
C. Merges from common should get reviewed early and approved<BR>
readily in order to avoid project divergence<BR>
<BR>
V. Changes to openstack-common.conf are a special case.<BR>
A. Patches with openstack-common.conf changes should include the<BR>
relevant merge-from-common changes.<BR>
B. Such patches should /not/ include any other merge-from-common<BR>
changes.<BR>
C. Such patches should not only include the common merges, but<BR>
should also include whatever code changes make use of the new merge.<BR>
D. Such patches require the same justification and scrutiny as any<BR>
other standard project patch.<BR>
<BR>
Please discuss! If we're able to reach general agreement about this<BR>
process, I will document the process in the openstack-common HACKING<BR>
guidelines.<BR>
<BR>
-Andrew<BR>
<BR>
<BR>
<BR>
<BR>
On 7/2/12 11:38 AM, Russell Bryant (Code Review) wrote:<BR>
> I did that before seeing this patch. However, I think I prefer what I pushed since it's individual updates explaining what they update instead of a blanket "update everything" commit.<BR>
><BR>
><BR>
<BR>
<BR>
_______________________________________________<BR>
Mailing list: <a href="https://launchpad.net/~openstack">https://launchpad.net/~openstack</a><BR>
Post to : <a href="openstack@lists.launchpad.net">openstack@lists.launchpad.net</a><BR>
Unsubscribe : <a href="https://launchpad.net/~openstack">https://launchpad.net/~openstack</a><BR>
More help : <a href="https://help.launchpad.net/ListHelp">https://help.launchpad.net/ListHelp</a><BR>
<BR>
</SPAN></FONT></BLOCKQUOTE>
</BODY>
</HTML>