[openstack-dev] [OpenStack-Dev] Cherry picking commit from oslo-incubator
Ben Nemec
openstack at nemebean.com
Tue Jan 21 19:13:55 UTC 2014
On 2014-01-21 12:28, Sean Dague wrote:
> On 01/21/2014 01:14 PM, Joe Gordon wrote:
>>
>> On Jan 17, 2014 12:24 AM, "Flavio Percoco" <flavio at redhat.com
>> <mailto:flavio at redhat.com>> wrote:
>>>
>>> On 16/01/14 17:32 -0500, Doug Hellmann wrote:
>>>>
>>>> On Thu, Jan 16, 2014 at 3:19 PM, Ben Nemec <openstack at nemebean.com
>> <mailto:openstack at nemebean.com>> wrote:
>>>>
>>>> On 2014-01-16 13:48, John Griffith wrote:
>>>>
>>>> Hey Everyone,
>>>>
>>>> A review came up today that cherry-picked a specific commit
>>>> to
>> OSLO
>>>> Incubator, without updating the rest of the files in the
>> module. I
>>>> rejected that patch, because my philosophy has been that when
>>>> you
>>>> update/pull from oslo-incubator it should be done as a full
>> sync of
>>>> the entire module, not a cherry pick of the bits and pieces
>> that you
>>>> may or may not be interested in.
>>>>
>>>> As it turns out I've received a bit of push back on this, so
>> it seems
>>>> maybe I'm being unreasonable, or that I'm mistaken in my
>> understanding
>>>> of the process here. To me it seems like a complete and
>>>> total
>> waste
>>>> to have an oslo-incubator and common libs if you're going to
>>>> turn
>>>> around and just cherry pick changes, but maybe I'm completely
>> out of
>>>> line.
>>>>
>>>> Thoughts??
>>>>
>>>>
>>>> I suppose there might be exceptions, but in general I'm with you.
>> For one
>>>> thing, if someone tries to pull out a specific change in the Oslo
>> code,
>>>> there's no guarantee that code even works. Depending on how the
>> sync was
>>>> done it's possible the code they're syncing never passed the Oslo
>>>> unit
>>>> tests in the form being synced, and since unit tests aren't
>>>> synced
>> to the
>>>> target projects it's conceivable that completely broken code
>>>> could get
>>>> through Jenkins.
>>>>
>>>> Obviously it's possible to do a successful partial sync, but for
>> the sake
>>>> of reviewer sanity I'm -1 on partial syncs without a _very_ good
>> reason
>>>> (like it's blocking the gate and there's some reason the full
>> module can't
>>>> be synced).
>>>>
>>>>
>>>> I agree. Cherry picking a single (or even partial) commit really
>> should be
>>>> avoided.
>>>>
>>>> The update tool does allow syncing just a single module, but that
>> should be
>>>> used very VERY carefully, especially because some of the changes
>> we're making
>>>> as we work on graduating some more libraries will include
>>>> cross-dependent
>>>> changes between oslo modules.
>>>
>>>
>>> Agrred. Syncing on master should be complete synchornization from
>>> Oslo
>>> incubator. IMHO, the only case where cherry-picking from oslo should
>>> be allowed is when backporting patches to stable branches. Master
>>> branches should try to keep up-to-date with Oslo and sync everything
>>> every time.
>>
>> When we started Oslo incubator, we treated that code as trusted. But
>> since then there have been occasional issues when syncing the code. So
>> Oslo incubator code has lost *my* trust. Therefore I am always a
>> hesitant to do a full Oslo sync because I am not an expert on the Oslo
>> code and I risk breaking something when doing it (the issue may not
>> appear 100% of the time too). Syncing code in becomes the first time
>> that code is run against tempest, which scares me.
>>
>> I would like to propose having a integration test job in Oslo
>> incubator
>> that syncs in the code, similar to how we do global requirements.
>
> That would be great, especially given that oslo currently doesn't run
> it's own unit tests in parallel, which means it's a lot easier for non
> parallel safe code to get in, and we don't bring the unit tests over to
> the projects on sync.
>
> Getting preemptive and doing a forward sync for validation would be
> really valuable.
>
> -Sean
That would be nice, but if we could do that then oslo wouldn't be an
incubating project. A fair number of the commits to oslo are
incompatible with the current state of the consuming projects and
require changes to work. A sync job would have to be non-voting at
best, and would be broken most, if not all, of the time while we wait to
get the necessary changes into all of the consuming projects.
-Ben
More information about the OpenStack-dev
mailing list