[openstack-dev] Code review study

Daniel P. Berrange berrange at redhat.com
Thu Aug 15 14:56:00 UTC 2013


On Thu, Aug 15, 2013 at 09:46:07AM -0500, Dolph Mathews wrote:
> On Thu, Aug 15, 2013 at 7:57 AM, Christopher Yeoh <cbkyeoh at gmail.com> wrote:
> 
> > On Thu, Aug 15, 2013 at 9:54 PM, Daniel P. Berrange <berrange at redhat.com>wrote:Commit message quality has improved somewhat since I first wrote &
> > published
> >
> >  that page, but there's definitely still scope to improve things further.
> >> What
> >> it really needs is for more reviewers to push back against badly written
> >> commit messages, to nudge authors into the habit of being more verbose in
> >> their commits.
> >>
> >>
> > Agreed. There is often "what" and sometimes "why", but not very often
> > "how" in commit messages.
> >
> 
> ++
> 
> Beyond the one line summary (which *should* describe "what" changed),
> describing "what" changed in the commit message is entirely redundant with
> the commit itself.

It isn't that clearcut actually. It is quite often helpful to summarize
"what" changed in the commit message, particularly for changes touching
large areas of code, or many files. The diff's can't always be assumed
to be easily readable - for example if you re-indented a large area of
code, the actual "what" can be clear as mud. Or if there are related
changes spread across many files & functions, a description of what is
being done will aid reviewers. Just be pragmatic about deciding when
a change is complex enough that it merits summarizing the 'what', as
well as the 'why'.


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 :|



More information about the OpenStack-dev mailing list