Re: checkkpatch (in)sanity ?

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 16:20:52 -0700	[thread overview]
Message-ID: <1472426452.26978.151.camel@perches.com> (raw)
In-Reply-To: <20160828223759.GA12993@sasha-lappy>

On Sun, 2016-08-28 at 18:37 -0400, Levin, Alexander wrote:
> On Sun, Aug 28, 2016 at 01:15:57PM -0400, Joe Perches wrote:
> > 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.
> I think that we need to figure out this disagreement first then. My
> claim is that checkpatch's output isn't useful.
[]
> It'll be interesting to hear from these people about their view of
> checkpatch, but IMO when on average there are more issues than commits
> I can suggest two possible causes:
> 
>  1. People are used to ignore checkpatch warnings.
>  2. People aren't using checkpatch.
> 
> Can you really make the claim that this is how checkpatch is supposed
> to be working?

<shrug>.  I make no particular claims about checkpatch.

I think checkpatch isn't particularly useful for those
thoroughly inculcated in what style the kernel uses and
is more useful for infrequent or new submitters.

The long time submitters and key maintainers are already
pretty consistent about coding style.

It would be good to examine the specific messages though.

For instance, Thomas Gleixner's messages for 200 commits:

     81 WARNING:COMMIT_LOG_LONG_LINE
     71 WARNING:BAD_SIGN_OFF
     37 WARNING:LONG_LINE
     22 WARNING:AVOID_BUG
     17 ERROR:SPACING
     16 WARNING:TYPO_SPELLING
     13 WARNING:UNSPECIFIED_INT
      7 ERROR:GIT_COMMIT_ID
      6 WARNING:MINMAX
      2 WARNING:PREFER_PR_LEVEL
      2 WARNING:LONG_LINE_COMMENT
      2 WARNING:LEADING_SPACE
      2 WARNING:FILE_PATH_CHANGES
      2 ERROR:CODE_INDENT
      1 WARNING:SUSPECT_CODE_INDENT
      1 WARNING:SPACING
      1 WARNING:ONE_SEMICOLON
      1 WARNING:LINE_SPACING
      1 WARNING:BLOCK_COMMENT_STYLE
      1 ERROR:TRAILING_STATEMENTS
      1 ERROR:INITIALISED_STATIC
      1 CHECK:PARENTHESIS_ALIGNMENT

28 ERRORs and a lot more WARNINGs

It seems that most of the BAD_SIGN_OFF uses are where
he signed his original patches and then did something
like git-am -s on those same patches.

The COMMIT_LOG_LONG_LINE messages are almost all just
slightly over the generic 80 column output when used
with just "git log".

I think the rest of the messages are reasonable and
generally follow CodingStyle.