Re: checkkpatch (in)sanity ?

From: "Levin, Alexander" <alexander.levin@verizon.com>
To: Joe Perches <joe@perches.com>
Cc: "Levin, Alexander" <alexander.levin@verizon.com>,
	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 22:22:13 -0400	[thread overview]
Message-ID: <20160829021055.GA24216@sasha-lappy> (raw)
In-Reply-To: <1472426452.26978.151.camel@perches.com>

On Sun, Aug 28, 2016 at 07:20:52PM -0400, Joe Perches wrote:
> 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.

I did the same test for authors of 5-9 commits (just an arbitrary choice of numbers for "infrequent"), the results there are much worse: 3981 commits, 7175 issues.

The only big subsystem that seems to be forcing checkpatch "correctness" is mm/, where akpm is fixing up checkpatch issues himself. Otherwise, it looks like maintainers are not running checkpatch nor are making sure that the commits they merge in don't have checkpatch issues.

> It would be good to examine the specific messages though.

What for? The point is that with that amount of issues it's evident that people don't actually use checkpatch to begin with. We can discuss whether the output it produces makes sense all we want, but the fact is that people just don't use it - and I've tried to give my opinion of why I think it happens.

-- 

Thanks,
Sasha