Open Stack

On Thu, Aug 15, 2013 at 9:56 AM, Daniel P. Berrange <berrange at redhat.com>wrote:

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


Acknowledged, but keyword: *summarize*! If it really won't fit in a one
line summary, that's a giant red flag that you should be producing multiple
commits.


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


Multiple commits!


> Or if there are related
> changes spread across many files & functions, a description of what is
> being done will aid reviewers.


This doesn't necessarily belong in the commit message... this can fall
under item #4 in the article -- annotate your code review before it begins.


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



-- 

-Dolph
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20130815/ccd7b8de/attachment.html>