<div dir="ltr"><br><div class="gmail_extra"><div class="gmail_quote">On Thu, Aug 15, 2013 at 9:56 AM, Daniel P. Berrange <span dir="ltr"><<a href="mailto:berrange@redhat.com" target="_blank">berrange@redhat.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On Thu, Aug 15, 2013 at 09:46:07AM -0500, Dolph Mathews wrote:<br>
> On Thu, Aug 15, 2013 at 7:57 AM, Christopher Yeoh <<a href="mailto:cbkyeoh@gmail.com">cbkyeoh@gmail.com</a>> wrote:<br>
><br>
> > On Thu, Aug 15, 2013 at 9:54 PM, Daniel P. Berrange <<a href="mailto:berrange@redhat.com">berrange@redhat.com</a>>wrote:Commit message quality has improved somewhat since I first wrote &<br>
> > published<br>
> ><br>
> >  that page, but there's definitely still scope to improve things further.<br>
> >> What<br>
> >> it really needs is for more reviewers to push back against badly written<br>
> >> commit messages, to nudge authors into the habit of being more verbose in<br>
> >> their commits.<br>
> >><br>
> >><br>
> > Agreed. There is often "what" and sometimes "why", but not very often<br>
> > "how" in commit messages.<br>
> ><br>
><br>
> ++<br>
><br>
> Beyond the one line summary (which *should* describe "what" changed),<br>
> describing "what" changed in the commit message is entirely redundant with<br>
> the commit itself.<br>
<br>
</div>It isn't that clearcut actually. It is quite often helpful to summarize<br>
"what" changed in the commit message, particularly for changes touching<br>
large areas of code, or many files.</blockquote><div><br></div><div>Acknowledged, but keyword: <i>summarize</i>! If it really won't fit in a one line summary, that's a giant red flag that you should be producing multiple commits.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The diff's can't always be assumed<br>
to be easily readable - for example if you re-indented a large area of<br>
code, the actual "what" can be clear as mud.</blockquote><div><br></div><div>Multiple commits!</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Or if there are related<br>
changes spread across many files & functions, a description of what is<br>
being done will aid reviewers.</blockquote><div><br></div><div>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.</div><div>
 </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Just be pragmatic about deciding when<br>
a change is complex enough that it merits summarizing the 'what', as<br>
well as the 'why'.<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
Daniel<br>
--<br>
|: <a href="http://berrange.com" target="_blank">http://berrange.com</a>      -o-    <a href="http://www.flickr.com/photos/dberrange/" target="_blank">http://www.flickr.com/photos/dberrange/</a> :|<br>
|: <a href="http://libvirt.org" target="_blank">http://libvirt.org</a>              -o-             <a href="http://virt-manager.org" target="_blank">http://virt-manager.org</a> :|<br>
|: <a href="http://autobuild.org" target="_blank">http://autobuild.org</a>       -o-         <a href="http://search.cpan.org/~danberr/" target="_blank">http://search.cpan.org/~danberr/</a> :|<br>
|: <a href="http://entangle-photo.org" target="_blank">http://entangle-photo.org</a>       -o-       <a href="http://live.gnome.org/gtk-vnc" target="_blank">http://live.gnome.org/gtk-vnc</a> :|<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div><br></div>-Dolph
</div></div>