Open Stack

On 11/20/2013 07:04 AM, Roman Bogorodskiy wrote:
> I know it was brought up on the list a number of times, but...
> 
> If we're talking about storing commit ids for each module and writing
> some shell scripts for that, isn't it a chance to reconsider using git
> submodules?

No. They're too complex. We don't allow merge commits because they're
too easy to mess up. Even seasoned (and I mean SEASONED git devs) shy
away from submodules because the semantics are tricky.

We have 1600 devs - advanced git features lead us to death. (I say this
as the person who fields questions about basic git features)

> On Wed, Nov 20, 2013 at 12:37 PM, Elena Ezhova <eezhova at mirantis.com
> <mailto:eezhova at mirantis.com>> wrote:
> 
> 
>     20.11.2013, 06:18, "John Griffith" <john.griffith at solidfire.com
>     <mailto:john.griffith at solidfire.com>>:
> 
> 
>     On Mon, Nov 18, 2013 at 3:53 PM, Mark McLoughlin <markmc at redhat.com
>     <mailto:markmc at redhat.com>> wrote:
> 
>     >  On Mon, 2013-11-18 at 17:24 +0000, Duncan Thomas wrote:
>     >>  Random OSLO updates with no list of what changed, what got fixed etc
>     >>  are unlikely to get review attention - doing such a review is
>     >>  extremely difficult. I was -2ing them and asking for more info, but
>     >>  they keep popping up. I'm really not sure what the best way of
>     >>  updating from OSLO is, but this isn't it.
>     >  Best practice is to include a list of changes being synced, for
>     example:
>     >
>     >    https://review.openstack.org/54660
>     >
>     >  Every so often, we throw around ideas for automating the
>     generation of
>     >  this changes list - e.g. cinder would have the oslo-incubator
>     commit ID
>     >  for each module stored in a file in git to tell us when it was last
>     >  synced.
>     >
>     >  Mark.
>     >
>     >  _____________________________
> 
>         __________________
>         >  OpenStack-dev mailing list
>         >  OpenStack-dev at lists.openstack.org
>         <mailto:OpenStack-dev at lists.openstack.org>
>         >  http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 
>         Been away on vacation so I'm afraid I'm a bit late on this... but;
> 
>         I think the point Duncan is bringing up here is that there are some
>         VERY large and significant patches coming from OSLO pulls.  The DB
>         patch in particular being over 1K lines of code to a critical
>         portion
>         of the code is a bit unnerving to try and do a review on.  I realize
>         that there's a level of trust that goes with the work that's done in
>         OSLO and synchronizing those changes across the projects, but I
>         think
>         a few key concerns here are:
> 
>         1. Doing huge pulls from OSLO like the DB patch here are nearly
>         impossible to thoroughly review and test.  Over time we learn a lot
>         about real usage scenarios and the database and tweak things as
>         we go,
>         so seeing a patch set like this show up is always a bit
>         unnerving and
>         frankly nobody is overly excited to review it.
> 
>         2. Given a certain level of *trust* for the work that folks do
>         on the
>         OSLO side in submitting these patches and new additions, I think
>         some
>         of the responsibility on the review of the code falls on the OSLO
>         team.  That being said there is still the issue of how these changes
>         will impact projects *other* than Nova which I think is sometimes
>         neglected.  There have been a number of OSLO synchs pushed to Cinder
>         that fail gating jobs, some get fixed, some get abandoned, but in
>         either case it shows that there wasn't any testing done with
>         projects
>         other than Nova (PLEASE note, I'm not referring to this particular
>         round of patches or calling any patch set out, just stating a
>         historical fact).
> 
>         3. We need better documentation in commit messages explaining
>         why the
>         changes are necessary and what they do for us.  I'm sorry but in my
>         opinion the answer "it's the latest in OSLO and Nova already has it"
>         is not enough of an answer in my opinion.  The patches mentioned in
>         this thread in my opinion met the minimum requirements because
>         they at
>         least reference the OSLO commit which is great.  In addition I'd
>         like
>         to see something to address any discovered issues or testing
>         done with
>         the specific projects these changes are being synced to.
> 
>         I'm in no way saying I don't want Cinder to play nice with the
>         common
>         code or to get in line with the way other projects do things but
>         I am
>         saying that I think we have a ways to go in terms of better
>         communication here and in terms of OSLO code actually keeping in
>         mind
>         the entire OpenStack eco-system as opposed to just changes that were
>         needed/updated in Nova.  Cinder in particular went through some
>         pretty
>         massive DB re-factoring and changes during Havana and there was
>         a lot
>         of really good work there but it didn't come without a cost and the
>         benefits were examined and weighed pretty heavily.  I also think
>         that
>         some times the indirection introduced by adding some of the
>         openstack.common code is unnecessary and in some cases makes things
>         more difficult than they should be.
> 
>         I'm just not sure that we always do a very good ROI investigation or
>         risk assessment on changes, and that opinion applies to ALL
>         changes in
>         OpenStack projects, not OSLO specific or anything else.
> 
>         All of that being said, a couple of those syncs on the list are
>         outdated.  We should start by doing a fresh pull for these and if
>         possible add some better documentation in the commit messages as to
>         the justification for the patches that would be great.  We can
>         take a
>         closer look at the changes and the history behind them and try
>         to get
>         some review progress made here.  Mark mentioned some good ideas
>         regarding capturing commit ID's from synchronization pulls and I'd
>         like to look into that a bit as well.
> 
>         Thanks,
>         John
> 
>         _______________________________________________
>         OpenStack-dev mailing list
>         OpenStack-dev at lists.openstack.org
>         <mailto:OpenStack-dev at lists.openstack.org>
>         http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 
> 
> 
>     I see now that updating OSLO is a far more complex issue than it may
>     seem from the first sight.
>     But I would really like to do my best to make it look acceptable and
>     possible to review.
> 
>     It seems everybody agrees that commit messages should have change
>     logs of the updated files.
>     It can be done by writing a shell script that will generate this
>     logs by simply executing a git log command.
>     But the problem is how to get the initial version of the file that
>     is being updated.
> 
>     As Mark McLoughlin said, it may be a good idea to store OSLO
>     commit-ids for each module in some file.
>     This file will have to be changed every time an update is performed
>     which implies changing update.py
>     in oslo-incubator. The the shell script will just have to fetch a
>     change-id from this file.
> 
>     I will try to write the script and generate logs, but I'm not sure
>     about how can I put all these logs in
>     a commit message because one patch usually changes more than one
>     file and each file may have
>     quite a long log.
> 
>     So I would really appreciate any comments or pieces of advice.
> 
>     Thanks,
>     Elena
> 
>     _______________________________________________
>     OpenStack-dev mailing list
>     OpenStack-dev at lists.openstack.org
>     <mailto: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
>