Re: [PATCH] Remove errors caught by checkpatch.pl in kernel/kallsyms.c

[Julia Lawall]

On Mon, 16 Feb 2009, Ingo Molnar wrote:

> 
> * Stefan Richter <stefanr@xxxxxxxxxxxxxxxxx> wrote:
> 
> > On 2/15/2009 7:47 PM, Sam Ravnborg wrote:
> > > On Mon, Feb 16, 2009 at 12:04:36AM +0530, Manish Katiyar wrote:
> > >> Hi Ingo,
> > >> 
> > >> I used your code-quality script to do cleanup in kernel/kallsyms.c.
> > >> Below patch removes errors generated by checkpatch.pl.
> > > When doing so use checkpatch only as a hint generator and do
> > > not concentrate only on the warnings/errors generated by checkpatch.
> > > 
> > > Your patch is an improvement but please fix the remaining issues.
> > 
> > Furthermore, the changelog is bad (non-exiting in fact).
> > 
> > The fact that the issues where discovered using checkpatch is absolutely
> > uninteresting.  The changelog should describe /what/ is fixed, e.g.
> > whitespace, maybe other things.  (In case of nontrivial changes the log
> > may also need to explain not only the /what but also the /how/, but this
> > does not apply to patches like this one.)
> 
> The commit log definitely needs enhancements but it's not uninteresting 
> at all what tools were used to arrive to a change. It shouldnt be in the 
> title, but can be mentioned in the changelog itself. (and should be 
> mentioned if the cleanup ever gets as far as the mainline kernel - if a 
> good and acceptable commit results out of a tool's usage then that tool 
> needs to be advertised some more.)

Is everything below the --- preserved in what is available via git log?  
Or at least git log -p. If so, perhaps it is indeed a good idea to move 
the "how" information there.  I think the how information has some value, 
both to make people aware of what tools are useful for what kinds of 
tasks, and to help one understand what criteria were used in making the 
patch.  This can sometimes be conveyed more precisely using code than 
English text.  I agree that the how information is not always relevant for 
the person who just wants to scan a changelog to see what is new in the 
current Linux release.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/