Open Stack

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.hellmann at dreamhost.com>wrote:

>
>
>
> On Wed, Jan 15, 2014 at 4:40 AM, Flavio Percoco <flavio at 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 <flavio at 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 at lists.openstack.org
>>>        http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>
>>>
>>>
>>>    _______________________________________________
>>>    OpenStack-dev mailing list
>>>    OpenStack-dev at lists.openstack.orghttp://lists.openstack.org/
>>> cgi-bin/mailman/listinfo/openstack-dev
>>>
>>>
>>>
>>>
>>>
>>  _______________________________________________
>>> OpenStack-dev mailing list
>>> OpenStack-dev at lists.openstack.org
>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>
>>
>>
>> --
>> @flaper87
>> Flavio Percoco
>>
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>>
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140115/87441d86/attachment.html>