On Wed, Jan 15, 2014 at 1:53 PM, Brant Knudson <[email protected]> wrote: > > 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. >
The first few steps of that seem like something that could be automated with a fairly simple script. > 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. > That makes sense to me. The request from the summit was to include the commits, but I think so far everyone is comfortable with backing that off to just say the most recent hash. Doug > > 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 < > [email protected]> wrote: > >> >> >> >> On Wed, Jan 15, 2014 at 4:40 AM, Flavio Percoco <[email protected]>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 <[email protected]> >>>> 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 >>>> [email protected] >>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/ >>>> openstack-dev >>>> >>>> >>>> >>>> _______________________________________________ >>>> OpenStack-dev mailing list >>>> [email protected]http://lists.openstack.org/ >>>> cgi-bin/mailman/listinfo/openstack-dev >>>> >>>> >>>> >>>> >>>> >>> _______________________________________________ >>>> OpenStack-dev mailing list >>>> [email protected] >>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>>> >>> >>> >>> -- >>> @flaper87 >>> Flavio Percoco >>> >>> _______________________________________________ >>> OpenStack-dev mailing list >>> [email protected] >>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>> >>> >> >> _______________________________________________ >> OpenStack-dev mailing list >> [email protected] >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >> >> > > _______________________________________________ > OpenStack-dev mailing list > [email protected] > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > >
_______________________________________________ OpenStack-dev mailing list [email protected] http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
