A quick guide to why stand-alone checkpatch patches suck...

[Robert P. J. Day]

On Tue, 16 Sep 2014, Greg KH wrote:

> On Tue, Sep 16, 2014 at 11:42:18PM -0400, Valdis.Kletnieks at vt.edu wrote:
> > (And this sort of analysis is exactly *why* people need to apply their brains
> > when looking at checkpatch output....)
>
> No one has ever said that they shouldn't.
>
> Remember, I know _lots_ of kernel developers who started with just
> "checkpatch cleanups on staging drivers" and they moved on to much
> "higher" roles in the kernel developer ecosystem (jobs, maintainers
> of subsystems, keynote talks at conferences, etc.)
>
> Don't "po po" it as something that shouldn't be a valid place to
> start, it is, and is why I do the work to review all of the many
> thousands of staging patches every release cycle.
>
> No one is forcing you to write those patches, or read / review them,
> so don't discourage others to provide them either please.  I most
> certainly do not.

  so while i'm waxing philosophical, some other thoughts that occurred
to me that reflect on how i got started, and ways to become a more and
more useful contributor to the kernel for newbies (and i'm willing to
be corrected on any of this).

  first, stop with the blind running of checkpatch unless you're
willing to take the results and examine the *context* in which they
occur. that file needs blank lines? ok, does it reside in a directory
of related files that could *also* use some blank lines? then do them
*all* -- don't waste peoples' time fixing one file at a time. if you
can do the same stylistic cleanup on an entire subsystem, go for it,
and don't clutter up the git log with numerous trivial commits.

  *but* ... don't try to sneak functional changes in there at the same
time. if it's a style cleanup, then it's a *style* cleanup. one thing
at a time. but there are other places you can make a name.

  first, read the Documentation/ directory -- there's lots of content
there, and quite a bit of it is out of date or just plain obsolete.
and if you want people to love you, improve the documentation. but,
see, that's going to take some work. and that's because it requires
you to read the documentation, then go off and examine the
corresponding code to see if it still matches. and why is this good?

  because while updating the Documentation/ content is safe and can't
break anything, the side effect is that you *learn* about that
particular subsystem, you get some nifty patches into the kernel, and
you make lots and lots of friends.

  another place to get cheap patches is to repair any kernel-doc
warnings, and there are *always* plenty of those. again, fixing
kernel-doc content shouldn't break anything, it should be easy
patching, and it normally requires you to at least examine the code to
make sure you're fixing it properly. so, you get patches into the
kernel, and you learn a bit more about some code. win-win.

  last point here regarding something gregkh wrote -- yes, it's fine
to *start* with simple stylistic cleanup, especially if checkpatch
does all the work for you. but remember, that's low-hanging fruit, and
you shouldn't be greedy and try and do all of it. if stylistic cleanup
is a way for beginners to get their first patches into the kernel,
then don't be a pig and try to do it all -- leave some for others to
cut their teeth on. and what is the point of all this?

  quite simply, this is also why nick krause will never be a useful
member of the kernel community. i suggested a while back that nick
could start with improving the documentation, for all the reasons i
mentioned above. his response was that he didn't know enough to do
that, which is an astonishing thing to admit. if you don't know enough
to improve the basic documentation, you have no right to be mucking
around in the code.

  and, as we've all seen, nick's other flaw is that, quite simply,
he's selfish and greedy. his entire obsession is with the output of
checkpatch, which means he wants to grab all the trivial cleanup (the
low-hanging fruit, as it were) for himself, and not leave any for
others. rather than take the time to understand the code, nick wants
checkpatch to do all the work for him. in the end, nick doesn't want
to do any work or understand how the kernel actually works -- he just
wants patches, and he wants them as quickly and cheaply as possible.

  anyway, it's time for coffee.

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================