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" <fla...@redhat.com
<mailto:fla...@redhat.com>> wrote:

On 16/01/14 17:32 -0500, Doug Hellmann wrote:

On Thu, Jan 16, 2014 at 3:19 PM, Ben Nemec <openst...@nemebean.com
<mailto:openst...@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

_______________________________________________
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Reply via email to