linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: "Levin, Alexander" <alexander.levin@verizon.com>
Cc: Sasha Levin <levinsasha928@gmail.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	"ksummit-discuss@lists.linuxfoundation.org" 
	<ksummit-discuss@lists.linuxfoundation.org>
Subject: Re: checkkpatch (in)sanity ?
Date: Sun, 28 Aug 2016 10:15:57 -0700	[thread overview]
Message-ID: <1472404557.26978.84.camel@perches.com> (raw)
In-Reply-To: <20160828023807.GC19088@sasha-lappy>

On Sat, 2016-08-27 at 22:47 -0400, Levin, Alexander wrote:

> Would you agree that by default we shouldn't show anything that's
> not an error/defect?

Not particularly, no.

> That doesn't deal with newlines people add to hide the 80 character stuff, nor it
> deals with the "harder to read" part.

Harder to read is almost all habituation.

80 columns is simply silly when dealing with either
long identifiers or many levels of indentation.

One thing that 80 column limit does do is encourage
shorter identifiers and fewer levels of indentation.

Generally, both of those are good things.

> > > By default you should only get the most critical warnings we have in the
> > > kernel like missing S-O-B or corrupt patch.
> > I don't think so, but if you do, add a filter for ERROR only.
> I could, but the problem is the people who see the default output as "holy".

Personally, I think the "my first kernel patch" beginners were
overly encouraged to produce these checkpatch whitespace type
changes by a couple things:

o Greg KH's TuxRadar article back in 2010
  http://www.tuxradar.com/content/newbies-guide-hacking-linux-kernel
o The Eudyptula Challenge
  http://eudyptula-challenge.org/

I don't know if the Eudyptula scripts are specific to
drivers/staging and most of those beginners haven't read his
email from 2015 that essentially says "don't do that" on
anything other than drivers/staging.

https://lists.kernelnewbies.org/pipermail/kernelnewbies/2015-July/014699.html

Outreach is hard.  Those efforts were perhaps worthwhile, but
has even a single productive kernel developer been produced
from one of those two outreach efforts?

> > > 2. A "who wrote these rules?": there seems to be a disconnect between the rules
> > > checkpatch is trying to enforce and the accepted coding style enforced by
> > > maintainers. 
> > Name some please.
> Well look at the git commit id SHA1 length thingie for example (GIT_COMMIT_ID).
> checkpatch says 12 chars minimum, but as far as I can tell Linus and Greg didn't get the memo.

That bit is in Documentation/SubmttingPatches

12 was Linus' length after the original 7 was too short.
12 will still be long enough for a few years yet.
https://lkml.org/lkml/2010/10/28/287

> > I think almost all of it is regexes and most people
> > aren't very good at those.
> > 
> > So it wouldn't matter if it was perl or python.
> > 
> > spatch isn't the right tool.
> > 
> > What would you suggest instead?
> This is a good topic to talk about, making checkpatch accessible to us
> commoners could be useful, we just need to figure out how.

I'm not sure that matters much at all.

I'm sure if you tried, you could produce that
"ERRORS" only patch for checkpatch.

There would still be some issues categorizing the
various tests at the appropriate level to taste.

  reply	other threads:[~2016-08-28 17:16 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-27 20:40 checkkpatch (in)sanity ? Joe Perches
2016-08-28  1:06 ` Levin, Alexander
2016-08-28  1:42   ` Joe Perches
2016-08-28  2:20     ` Joe Perches
2016-08-28  2:47     ` Levin, Alexander
2016-08-28 17:15       ` Joe Perches [this message]
2016-08-28 17:59         ` Greg KH
2016-08-28 22:37         ` Levin, Alexander
2016-08-28 23:20           ` Joe Perches
2016-08-29  2:22             ` Levin, Alexander
2016-08-29  8:20               ` Christoph Hellwig
2016-08-29  7:15           ` [Ksummit-discuss] " Alexandre Belloni
2016-08-29  9:01             ` Arnd Bergmann
2016-08-29 12:47               ` Joe Perches
2016-08-29 17:16                 ` Josh Triplett
2016-08-29 17:41                   ` Joe Perches
2016-08-29 17:46         ` Luck, Tony
2016-08-29 18:01           ` Joe Perches
2016-08-29 18:47             ` Joe Perches
2016-08-29 19:08             ` Josh Triplett
2016-08-29 21:07             ` Arnd Bergmann
2016-08-29 11:15     ` Kalle Valo
2016-08-29 12:30       ` Joe Perches
2016-08-29 18:01         ` Kalle Valo
2016-08-29 19:00           ` Joe Perches
2016-08-29 21:00           ` [Ksummit-discuss] " Arnd Bergmann
2016-08-28  7:56   ` Alexey Dobriyan
2016-08-28  9:59     ` Julia Lawall
2016-08-28 19:52       ` Joe Perches
2016-08-28 20:35         ` Jiri Kosina
2016-08-28 21:24         ` Dennis Kaarsemaker
2016-08-28 21:57           ` Joe Perches
2016-08-29 19:06 ` Dan Carpenter
2016-08-29 19:10   ` Josh Triplett
2016-08-29 19:17     ` Dan Carpenter
2016-08-29 19:34       ` Joe Perches
2016-08-29 19:21     ` Joe Perches

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=1472404557.26978.84.camel@perches.com \
    --to=joe@perches.com \
    --cc=alexander.levin@verizon.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=ksummit-discuss@lists.linuxfoundation.org \
    --cc=levinsasha928@gmail.com \
    --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;
as well as URLs for NNTP newsgroup(s).