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 <[email protected] > <mailto:[email protected]>> wrote: > > > 20.11.2013, 06:18, "John Griffith" <[email protected] > <mailto:[email protected]>>: > > > On Mon, Nov 18, 2013 at 3:53 PM, Mark McLoughlin <[email protected] > <mailto:[email protected]>> 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 > > [email protected] > <mailto:[email protected]> > > 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 > [email protected] > <mailto:[email protected]> > 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 > [email protected] > <mailto:[email protected]> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > > > > > _______________________________________________ > OpenStack-dev mailing list > [email protected] > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > _______________________________________________ OpenStack-dev mailing list [email protected] http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
