Re: [PATCH] checkpatch: Warn on empty commit log bodies

[Theodore Tso <tytso@mit.edu>]

On Mon, Mar 02, 2009 at 07:19:11PM +0000, Mark Brown wrote:
> I'm not sure that simply supplying a body would help anything here, FWIW
> - if I'm writing a body for the sake of it it'll generally just be
> repetitive which isn't terribly constructive.  Obviously I do *try* to
> write sensible changelogs and will keep making an effort to do so.  As
> far as I remember most of the issues in the past have been due to
> missing out something along the lines of "...because that's what the
> silicon does" but ICBW.

If you mean stuff like this:

    commit a39a021fd73ce06aad8d1081ac711a36930e6cb8
    Author: Mark Brown <broonie@opensource.wolfsonmicro.com>
    Date:   Wed Feb 4 21:10:58 2009 +0100

        mfd: Mark WM835x USB_SLV_500MA bit as accessible
        
        The code is out of sync with the silicon.
        
        Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
        Signed-off-by: Samuel Ortiz <sameo@openedhand.com>

What I wouldn't know, looking at the message, and what I suspect
Andrew is complaining about, is after reading the entire commit log,
"So what?"  Is it going to cause the the device to explode?  Will the
system crash?  Will the device go nonfunctional until you power cycle
it?  What will the user see before this patch is applied?

Maybe it's obvious to you, since you know the device, but it's not
obvious to the rest of us.   This in contrast to a commit log such as:

subsys: Add error checking to kmalloc() call

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

... where all good kernel developers should understand the
consequences of not checking the return value to kmalloc().  But do
you really expect most kernel developers to know what the heck a
USB_SLV_500MA bit means, and what it means for it to be "accessible"
versus "not accessible"?

   "Always write with a deep sympathy for the reader"
   	   	      	     Will Strunk, Elements of Style

		     	  	  	   - Ted