Open Stack

I don't recall the full discussion from before, but I know one of the
big problems with doing that is it actually makes it more difficult to
review these syncs. Instead of having 1000 lines of copied changes to
review, you have a one-line commit hash to look at and you then have to
try to figure out what changed since the previous hash, whether that's
one line or 1000. 

I think there were some other issues too, but I don't remember what they
were so maybe someone else can chime in. 

-Ben 

On 2013-11-20 06:04, 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?
> 
> On Wed, Nov 20, 2013 at 12:37 PM, Elena Ezhova <eezhova at mirantis.com> wrote:
> 
> 20.11.2013, 06:18, "John Griffith" <john.griffith at solidfire.com>: 
> 
> On Mon, Nov 18, 2013 at 3:53 PM, Mark McLoughlin <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 [1]
>>
>> 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
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev [2]
> 
> 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
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev [2] 
> 
> 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
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev [2]

_______________________________________________
OpenStack-dev mailing list
OpenStack-dev at lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev [2]

 

Links:
------
[1] https://review.openstack.org/54660
[2] 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/20131120/bdc9f435/attachment.html>