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