On Thu, Jun 28, 2012 at 12:01:10PM +0200, Thierry Carrez wrote: > Daniel P. Berrange wrote: > > [...] > > In other words, when reviewing a change in Gerrit, do not simply look at > > the correctness of the code. Review the commit message itself and request > > improvements to its content. Look out for commits which are mixing multiple > > logical changes and require the submitter to split them into separate commits. > > Ensure whitespace changes are not mixed in with functional changes. Ensure > > no-op code refactoring is done separately from functional changes. And so > > on. > > [...] > > Nice work, and agreed on all points ! I particularly hate the > single-line "Fixes bug 1234566"-type commit messages. > > Is there a way a concise version of this advice could find its way into > HACKING.rst ? And/Or into http://wiki.openstack.org/ReviewChecklist ? Sure, MarkMc suggested to me that I put this doc up on the wiki somewhere. I'll do that and then submit a concise version for HACKING.rst and the ReviewChecklist page, with a cross-reference to the full thing. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|