Re: "CodingStyle: Clarify and complete chapter 7" in docs-next
From: Joe Perches
Date: Thu Sep 22 2016 - 06:42:31 EST
On Thu, 2016-09-22 at 11:24 +0200, Jean Delvare wrote:
[]
> > The seriousness with which some beginners take these message
> > types though is troublesome,
[]
> You need to think in terms of actual use cases. Who uses checkpatch and
> why? I think there are 3 groups of users:
> * Beginners. They won't run the script by themselves, instead they will
> submit a patch which infringes a lot of coding style rules, and the
> maintainer will point them to checkpatch and ask for a resubmission
> which makes checkpatch happy. Being beginners, they can only rely on
> the script itself to only report things which need to be fixed, by
> default.
> * Experienced developers. Who simply want to make sure they did not
> overlook anything before they post their work for review. They have
> the knowledge to decide if they want to ignore some of the warnings.
> * People with too much spare time, looking for anything they could
> "contribute" to the kernel. They will use --subjective and piss off
> every maintainer they can find.
I think you overlook the category of a beginner submitting
"my first kernel patch" which is a "coding style" defect of
some type. The Eudyptula and Outreachy programs seem to
encourage these sorts of patches.
This is where "scripts/checkpatch.pl -f <file>" is most used.
I believe adding the --force option might be useful to
restrict cleanup-style-only patches outside of staging.
There's nothing wrong with cleanup style patches, it can be
good introduction to compiler/config tool & kernel setup.
> I would rather suggest:
>
> ERROR -> MUST_FIX
> WARNING -> SHOULD_FIX
> CHECK -> MAY_FIX
MUST is much stronger language than I would prefer.
There are still about a quarter million ERRORs just for
spacing issues in the kernel tree.
Here are the top 10 ERROR checkpatch messages treewide as of
a few days ago,
$ grep ERROR checkpatch.short_sorted_20160917
268308 ERROR:SPACING
37340 ERROR:CODE_INDENT
27678 ERROR:TRAILING_WHITESPACE
21024 ERROR:COMPLEX_MACRO
14048 ERROR:POINTER_LOCATION
12207 ERROR:TRAILING_STATEMENTS
11079 ERROR:OPEN_BRACE
6802 ERROR:ASSIGN_IN_IF
3940 ERROR:RETURN_PARENTHESES
2322 ERROR:NON_OCTAL_PERMISSIONS
Maybe there could be some better classifications of the various
messages.
But there are about two million checkpatch messages overall in
the kernel tree.
That's a lot.