mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: checkpatch guide for newbies
Date: Mon, 23 Sep 2013 11:06:51 -0700	[thread overview]
Message-ID: <1379959611.3575.40.camel@joe-AO722> (raw)
In-Reply-To: <20130923090100.GE6192@mwanda>

On Mon, 2013-09-23 at 12:01 +0300, Dan Carpenter wrote:
> I've written a checkpatch guide for newbies because it seems like they
> make the same mistakes over and over.  I intend to put it under
> Documentation/.  Could you look it over?
> 
> 
> 		Introduction
> 
> This document is aimed at new kernel contributors using "checkpatch.pl --file".
> 
> The first thing to remember is that the aim is not to get rid of every
> checkpatch.pl warning; the aim is to make the code cleaner and more readable.
> The other thing to remember is that checkpatch.pl is not a very smart tool and
> there are mistakes that it misses so keep your eyes open for those as well.

Maybe suggest using

	checkpatch.pl --file --fix --strict --types=<specific_type> <file>
	mv <file>.EXPERIMENTAL-checkpatch-fixes <file>
	git diff <file>

where <specific_types> is one of

For whitespace only changes
	SPACING
	SPACE_BEFORE_TAB
	POINTER_LOCATION
For code changes:
	C99_COMMENTS
	PREFER_PACKED
	PREFER_ALIGNED
	AVOID_EXTERNS
	PARENTHESIS_ALIGNMENT

> For example, checkpatch.pl could warn about a badly formatted warning message.
> Ask yourself, is the warning message is clear?  Is it needed?  Could a
> non-privileged user trigger the warning and flood the syslog?  Are there
> spelling mistakes?  Since Checkpatch.pl has flagged the line as sloppy code,
> there may be multiple mistakes.
> 
> In the Linux kernel, we take an enormous pride in our work and we want clean
> code.  But the one major drawback to cleanup patches is that they break
> "git blame" so it's not a good idea for newbies to send very trivial cleanup
> patches to the kernel/ directory.  It's better to get a little experience in the
> drivers/ directory first.  The drivers/staging/ directory in particular always
> needs cleanup patches.

I think suggesting working _only_ in the drivers/staging/
directory tree might be better.

Maybe I'll submit some auto-neatening script eventually
and see what you think.


  parent reply	other threads:[~2013-09-23 18:06 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-23  9:01 Dan Carpenter
2013-09-23 12:46 ` Peter Senna Tschudin
2013-09-23 13:29   ` Dan Carpenter
2013-09-23 14:04   ` Geert Uytterhoeven
2013-09-23 18:06 ` Joe Perches [this message]
2013-09-23 20:17   ` Joe Perches
2013-09-23 18:35 ` bojan prtvar
2013-09-24 16:36 ` Bjorn Helgaas
2013-09-24 17:26   ` Alexander Holler
2013-09-24 17:43     ` Alexander Holler
2013-09-24 18:50       ` Alexander Holler
2013-09-24 19:29     ` Peter Senna Tschudin
2013-09-24 19:59       ` Dan Carpenter
2013-09-24 20:13       ` Bjorn Helgaas
2013-09-24 22:10         ` Alexander Holler
2013-09-26  2:11           ` Al Viro
2013-09-26  2:52             ` Alexander Holler
2013-09-26  2:57               ` Alexander Holler
2013-09-26  3:04                 ` Al Viro
2013-09-26  3:27                   ` Alexander Holler
2013-09-26  3:48                     ` Al Viro
2013-09-26  4:21                       ` Alexander Holler
2013-09-26  5:53                     ` Julia Lawall
2013-09-26  9:55                       ` Alexander Holler

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=1379959611.3575.40.camel@joe-AO722 \
    --to=joe@perches.com \
    --cc=dan.carpenter@oracle.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.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

all inboxes | Powered by JetHome®