But what if I want to update some module that consists of ten or even more files (like rpc or db) and each of these files has quite a long change log? In that case the commit message may turn out to be really long even if only commit ids and names are included.
On Wed, Nov 20, 2013 at 12:37 PM, Elena Ezhova <eezh...@mirantis.com> wrote: > > 20.11.2013, 06:18, "John Griffith" <john.griff...@solidfire.com>: > > On Mon, Nov 18, 2013 at 3:53 PM, Mark McLoughlin <mar...@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 >> > OpenStackemail@example.com >> > 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 >> OpenStackfirstname.lastname@example.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 OpenStackemail@example.com http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev