kernelnewbies.kernelnewbies.org archive mirror
 help / color / mirror / Atom feed
From: rpjday@crashcourse.ca (Robert P. J. Day)
To: kernelnewbies@lists.kernelnewbies.org
Subject: A quick guide to why stand-alone checkpatch patches suck...
Date: Wed, 17 Sep 2014 06:39:12 -0400 (EDT)	[thread overview]
Message-ID: <alpine.LFD.2.11.1409170617270.26952@localhost> (raw)
In-Reply-To: <20140917045628.GB19159@kroah.com>

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.

  as someone who started out this way (submitting "trivial" patches,
and still do from time to time) and who now makes a living teaching
kernel programming and embedded linux and device drivers, perhaps i
can add some perspective, and also explain why nick krause is
monstrously off-base in everything he touches.

  of *course* it's useful that beginners get the opportunity to submit
trivial patches based on nothing but perhaps checkpatch warnings --
it's a great way to get your feet wet, burn in the lessons of how to
write and submit a proper patch, and so on and so on.  but notice the
really important point gregkh makes here:

> 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.)

the obvious implication is that, while you *start* simple, the goal is
to ***move up***. trivial, style-based patches are a great
*introduction*, but everyone should have the eventual goal of more and
more sophisticated patches and contributions involving tweaking code
and eventually writing new subsystems, etc, etc. and this is where
nick krause is failing miserably.

  nick shows absolutely no interest in understanding the code he's
looking at. his approach to patches is to blindly run checkpatch, look
at the first warning, go to that file, and try to "fix" it, without in
any way whatsoever trying to understand the code in a larger context.
if checkpatch says to add blank lines, nick will add blank lines,
after which he will understand no more about the code than when he
started, which is why, regardless of how long nick does this, he will
never, ever, ever understand any more about the kernel than he does
now.

  nick has made it obvious he has no interest in actually
understanding how the kernel works -- all he is obsessed with is
getting his name into the git log as the author of a patch; hence, his
relentless labour in submitting variation after variation of a patch
that does nothing more than add three blank lines to a single file.

  nick has long since lost sight of what that single source file is
doing (if he ever even cared what it did in the first place). he is
now in a very unhealthy place where he is going to get those blank
lines in there if it kills him or pisses off every single person on
the kernelnewbies mailing list, and that is precisely why working with
him is a complete waste of time.

  other beginners will start where nick is now and, in short order,
they will progress to bigger and better things -- as greg kh suggests,
writing code, becoming subsystem maintainers, giving keynotes. nick
will never, ever, ever do any of this; five years from now, nick will
still blindly be running checkpatch, then going to files looking for
blank lines to add. he will never progress beyond that, simply because
he's doing this for all the wrong reasons.

  nick doesn't care about how the kernel works. nick just wants to get
a patch in there somewhere ... anywhere, it doesn't matter. which is
why he is not worth anyone's time. nick will never be a useful
contributor to the kernel community because, in the end, he doesn't
really care about the kernel.

rday

-- 

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

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

  parent reply	other threads:[~2014-09-17 10:39 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-17  0:25 A quick guide to why stand-alone checkpatch patches suck Valdis Kletnieks
2014-09-17  0:37 ` Greg KH
2014-09-17  1:35 ` Greg Donald
2014-09-17  3:42   ` Valdis.Kletnieks at vt.edu
2014-09-17  4:56     ` Greg KH
2014-09-17  5:43       ` Sudip Mukherjee
2014-09-17 10:39       ` Robert P. J. Day [this message]
2014-09-17 11:20       ` Robert P. J. Day
2014-09-17 11:38         ` nick
2014-09-17 11:51           ` Sudip Mukherjee
2014-09-17 11:53             ` nick
2014-09-17 12:05               ` Greg Freemyer
2014-09-17 12:09                 ` nick
2014-09-17 12:17                   ` Kai Bojens
2014-09-17 12:23                     ` nick
2014-09-17 12:25                   ` Greg Freemyer
2014-09-17 12:29                     ` Nick Krause
2014-09-17 14:39                   ` Valdis.Kletnieks at vt.edu
2014-09-17 11:53           ` Robert P. J. Day
2014-09-17 11:55             ` nick
2014-09-17 12:17               ` Chris Lee
2014-09-17 12:19                 ` nick
2014-09-17 11:56         ` Greg Freemyer
2014-09-17 12:00           ` Robert P. J. Day
2014-09-17 12:05             ` nick
2014-09-17 12:02           ` nick
2014-09-17 14:39             ` Valdis.Kletnieks at vt.edu
2014-09-17 17:04               ` Nick Krause
2014-09-17 17:13               ` Bruno Guedes Souto
2014-09-17 17:47                 ` Nick Krause
2014-09-17 18:01                   ` Philipp Muhoray
2014-09-17 18:14                     ` Nick Krause
2014-09-17 18:15                   ` Robert P. J. Day
2014-09-17 18:44                     ` Nick Krause
2014-09-17 20:45                 ` John de la Garza

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.2.11.1409170617270.26952@localhost \
    --to=rpjday@crashcourse.ca \
    --cc=kernelnewbies@lists.kernelnewbies.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).