lkml.org 
[lkml]   [2009]   [Oct]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 3/5] checkpatch: add a blacklist
From
Date
On Wed, 2009-10-07 at 07:26 -0700, Daniel Walker wrote:
> On Wed, 2009-10-07 at 12:17 +0200, Krzysztof Halasa wrote:
> > Daniel Walker <dwalker@fifo99.com> writes:
> >
> > > This thread is specifically about checkpatch errors .. checkpatch
> > > warnings can be ignored, but errors you can't usually ignore..
> >
> > Of course I can and do :-)
> >
> > > If your
> > > ignoring errors then either checkpatch is producing bogus output that
> > > needs to be corrected, or it's something you really should fix..
> >
> > Neither.
> > But unfortunately I don't have examples handy.
> >
> > My POV must be a bit different: I treat errors like another class of
> > warnings (perhaps more important that "mere" warnings but still not
> > authoritative).
>
> >From my perspective Documentation/SubmittingPatches really dictates what
> you should be doing with checkpatch, since that was signed off on by
> Andy (on this thread) and Linus .. In that document I think checkpatch
> is given authority, rather than what your suggesting where it's just
> something you can use or not, and ignore or not like it has no meaning
> at all..

Daniel,

This is getting old. You've successfully entered the /dev/null folder to
several major developers.

The checkpatch.pl script is a very useful tool. I run it on all my
patches to make sure that I don't have any silly formatting errors. It
even catches some real bugs now and then.

That said, if we really wanted to have checkpatch as a authoritative
tool, it would be executed by a bot on all patches submitted to LKML
(which you seem to have put on yourself to do). But if Linus or others
wanted that, they would have set it up.

We assume that the maintainers of the system are competent enough to
keep a decent formatting style that conforms to the rest of the kernel.
There are some instances that the style may change to cover cases that
are unique, like the events headers.

Really it should be up to the maintainer to tell a submitter that they
need to run checkpatch. You are coming out as the checkpatch Nazi leader
to "enforce" your will of the tool on others. And when they tell you,
it's not that big of a deal, you have a conniption.

So my advice to you is to take a chill pill (they come in chewables) and
relax a bit on this topic. If you had just sent out some nice emails to
obvious breakage in patches, then it would have been fine. But you are
coming across a bit too authoritarian, and it is becoming quite
annoying.

-- Steve




\
 
 \ /
  Last update: 2009-10-07 17:21    [W:0.147 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site