<div dir="ltr"><div><div><div><div><div><div><br></div>Here's the process I use to review a oslo-incubator merge... since I'm mostly reviewing keystone I'm going to use that as the project.<br><br></div>1) Make sure keystone master is at the latest, and that oslo-incubator is at the right level (the commit hash if they mentioned it or latest)<br>
</div>2) run update.py in oslo-incubator, make sure that there's changes in keystone (maybe we have it already from a different review)<br></div>3) revert changes in keystone and check out the review with git-review -d<br>
</div>4) run update.py again and make sure there's no changes in keystone -- If there's changes then I -1 the review, since I can't verify it was done correctly.<br></div>5) Then I go through the review and look for problems... changes that aren't backwards compatible, new config options for the sample config file, etc.<br>
<div><div><br></div><div>At no point do I care what are the different commits that are being brought in from oslo-incubator. If the commits are listed in the commit message then I feel an obligation to verify that they got the right commits in the message and that takes extra time for no gain.<br>
<br>So I would prefer it if the message for syncs from oslo-incubator included the commit hash and did not include a list of changes.<br>
<br>If generating the list of changes was automated then I'd add a step to my process to verify the commit message was the same.<br></div><div><div><div><br></div><div>- Brant<br></div><div><br></div></div></div></div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Jan 15, 2014 at 6:44 AM, Doug Hellmann <span dir="ltr"><<a href="mailto:doug.hellmann@dreamhost.com" target="_blank">doug.hellmann@dreamhost.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_extra"><br><br><div class="gmail_quote">
<div class="im">On Wed, Jan 15, 2014 at 4:40 AM, Flavio Percoco <span dir="ltr"><<a href="mailto:flavio@redhat.com" target="_blank">flavio@redhat.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>On 14/01/14 16:33 -0600, Ben Nemec wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On 2014-01-14 15:26, Doug Hellmann wrote:<br>
<br>
In the release meeting today, Russell proposed that we at least include the<br>
hash of the HEAD when the merge is done, to indicate how far along the oslo<br>
changes are. More detail is obviously better.<br>
So, let's consider this as a new policy. Does anyone foresee issues with<br>
making this work?<br>
<br>
</blockquote>
<br></div>
+1 from me. Lets keep it simple. I agree with you on the fact that we<br>
should be focusing more on graduating modules from the incubator.<div><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I don't see a problem with doing that, but I'm not clear on where we're<br>
including the hash. In the file itself, in a separate file, and/or in the<br>
commit message?<br>
<br>
Even if we do no more automation, having the commit hash of the last sync would<br>
be immensely helpful. Not having to comb through commit logs to figure out<br>
when the last sync happened would be fantastic. :-)<br>
</blockquote>
<br></div>
We should keep it in the openstack-modules.conf file and put it in the<br>
commit message as well. IMHO.<br></blockquote><div><br></div></div><div><div class="gmail_default" style="font-size:small">I was thinking the commit message, but if you see usefulness in including the conf file, we could do that, too.</div>
<span class="HOEnZb"><font color="#888888">
<div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Doug</div><br></font></span></div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
FF<div><div><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
-Ben<br>
<br>
<br>
<br>
On Tue, Jan 14, 2014 at 4:23 AM, Flavio Percoco <<a href="mailto:flavio@redhat.com" target="_blank">flavio@redhat.com</a>> wrote:<br>
<br>
On 13/01/14 12:07 -0500, Doug Hellmann wrote:<br>
[.................]<br>
<br>
<br>
<br>
I spent some time trying to think through how we could improve the<br>
update<br>
script for [1], and I'm stumped on how to figure out *accurately*<br>
what state<br>
the project repositories are in today.<br>
<br>
We can't just compute the hash of the modules in the project<br>
receiving copies,<br>
and then look for them in the oslo-incubator repo, because we<br>
modify the files<br>
as we copy them out (to update the import statements and replace<br>
"oslo" with<br>
the receiving project name in some places like config option<br>
defaults).<br>
<br>
We could undo those changes before computing the hash, but the<br>
problem is<br>
further complicated because syncs are not being done of all modules<br>
together.<br>
The common code in a project doesn't move forward in step with the<br>
oslo-incubator repository as a whole. For example, sometimes only<br>
the openstack<br>
/common/log.py module is copied and not all of openstack/common. So<br>
log.py<br>
might be newer than a lot of the rest of the oslo code. The problem<br>
is even<br>
worse for something like rpc, where it's possible that modules<br>
within the rpc<br>
package might not all be updated together.<br>
<br>
We could probably spend a lot of effort building a tool to tell us<br>
exactly what<br>
the state of all of each common file is in each project, to figure<br>
out what<br>
needs to be synced. I would much rather spend that effort on<br>
turning the common<br>
code into libraries, though.<br>
<br>
So, here's an alternative:<br>
<br>
1. Projects accept a full sync of Oslo soon, including adding a<br>
value in their<br>
openstack-common.conf indicating which commit in oslo-incubator is<br>
reflected in<br>
the sync. We'll try to make those commit messages as detailed as<br>
possible.<br>
<br>
2. We modify update.py to remove the option to update individual<br>
modules when<br>
copying from oslo-incubator. The new version would always apply all<br>
changes<br>
from the last merged commit, as a series of commits, to the<br>
receiving project.<br>
So if nova is out of step by 3 commits, then 3 new commits would be<br>
created in<br>
the branch by the person doing the update, each with the commit log<br>
message<br>
from the change in oslo-incubator. (This lock-step approach is<br>
necessary to<br>
have any hope of figuring out which commits are actually being<br>
synced, so the<br>
log messages are accurate.)<br>
<br>
In my experience, when syncing files from oslo, it'll most likely<br>
require syncing more than one module. There's been just *few* times<br>
where copying a module from oslo resulted in just that specific module<br>
being copied.<br>
<br>
All that to say that I agree with this point.<br>
<br>
<br>
<br>
<br>
3. The person proposing the merge into the project can decide<br>
whether to squash<br>
the commits, or leave them as separate reviews.<br>
<br>
<br>
<br>
<br>
If we use relative imports for modules in oslo incubators (as<br>
mentioned in another email in this thread) and we *always* keep<br>
everything up to the latest. What about reconsidering using git<br>
submodules?<br>
<br>
AFAIR, the main issue with git submodules is that we wanted to support<br>
updating individual modules. If we remove that option, I think git<br>
submodules would work just fine. Am I missing something?<br>
<br>
Instead of hacking on update.py we could work on a migration plan out<br>
of it.<br>
<br>
A downside of using submodules is that when moving the reference in<br>
the submodule, it won't be obvious why that's happening, which is<br>
something we wanted to fix with update.py. It would be up to the<br>
committer to write a good commit message or to get the messages out of<br>
the submodule history.<br>
<br>
Another downside is that it would be hard to apply isolated patches on<br>
stable branches for security issues or really awful bugs.<br>
<br>
I'm less convinced about submodules now but I'm leaving this in the<br>
email in case someone wants to dig a bit deeper in the topic.<br>
<br>
<br>
<br>
I'm not entirely certain I like this approach myself, but it's the<br>
best I've<br>
been able to come up with. It essentially gives us the current<br>
process, while<br>
removing the ability to potentially take a version of a module<br>
without taking<br>
its dependencies (allowing us to step forward, and track the commit<br>
messages<br>
accurately). It will also produce results similar to what we will<br>
have when all<br>
of this oslo code moves into separate libraries, where the changes<br>
to the<br>
library will be seen by the projects without any action at all on<br>
their part.<br>
<br>
After going through this for a bit, I agree with you. The goal<br>
of the update script should be:<br>
<br>
- Sync modules from the current state to the most updated version<br>
<br>
- Make sure the update information is not lost. If there's an oslo<br>
sync review without the commits shas + description, it simply<br>
means the committer amended the message.<br>
<br>
<br>
<br>
OTOH, it will also require spending time on update.py, instead of<br>
releasing a<br>
library from the incubator. And it doesn't really buy us that much<br>
in terms of<br>
making the sync happen more easily, other than a reliable way of<br>
having<br>
entirely accurate commit messages.<br>
<br>
Although it distracts us from our real goal - releasing libraries - I<br>
still think is necessary. We should probably just reduce the changes<br>
needed as much as possible, but we'll need it anyway.<br>
<br>
<br>
<br>
I would love to have someone else offer an alternative that's less<br>
effort to<br>
change and provides the desired detailed log messages accurately.<br>
<br>
I think this actually simplifies the way update.py works, TBH. We'll<br>
be removing single module sync and we'll also force projects to be<br>
updated to the latest version of those modules in oslo-incubator,<br>
which is safer, IMHO.<br>
<br>
<br>
Cheers,<br>
FF<br>
<br>
--<br>
@flaper87<br>
Flavio Percoco<br>
<br>
______________________________<u></u>_________________<br>
OpenStack-dev mailing list<br>
<a href="mailto:OpenStack-dev@lists.openstack.org" target="_blank">OpenStack-dev@lists.openstack.<u></u>org</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/<u></u>cgi-bin/mailman/listinfo/<u></u>openstack-dev</a><br>
<br>
<br>
<br>
______________________________<u></u>_________________<br>
OpenStack-dev mailing list<br>
OpenStack-dev@lists.openstack.<u></u>orghttp://<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">lists.openstack.org/<u></u>cgi-bin/mailman/listinfo/<u></u>openstack-dev</a><br>
<br>
<br>
<br>
<br>
</blockquote>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
______________________________<u></u>_________________<br>
OpenStack-dev mailing list<br>
<a href="mailto:OpenStack-dev@lists.openstack.org" target="_blank">OpenStack-dev@lists.openstack.<u></u>org</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/<u></u>cgi-bin/mailman/listinfo/<u></u>openstack-dev</a><br>
</blockquote>
<br>
<br></div></div><span><font color="#888888">
-- <br>
@flaper87<br>
Flavio Percoco<br>
</font></span><br>_______________________________________________<br>
OpenStack-dev mailing list<br>
<a href="mailto:OpenStack-dev@lists.openstack.org" target="_blank">OpenStack-dev@lists.openstack.org</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br></blockquote></div></div></div><br></div></div>
<br>_______________________________________________<br>
OpenStack-dev mailing list<br>
<a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br></blockquote></div><br></div>