All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Jean Delvare <jdelvare@suse.de>
Cc: Julia Lawall <julia.lawall@lip6.fr>,
	Al Viro <viro@ZenIV.linux.org.uk>,
	Ilya Dryomov <idryomov@gmail.com>,
	Andy Whitcroft <apw@canonical.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Ceph Development <ceph-devel@vger.kernel.org>,
	Alex Elder <elder@kernel.org>, Sage Weil <sage@redhat.com>,
	LKML <linux-kernel@vger.kernel.org>,
	kernel-janitors@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-doc@vger.kernel.org
Subject: Re: "CodingStyle: Clarify and complete chapter 7" in docs-next
Date: Thu, 22 Sep 2016 03:42:10 -0700	[thread overview]
Message-ID: <1474540930.8253.9.camel@perches.com> (raw)
In-Reply-To: <20160922112407.47da9393@endymion>

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.

WARNING: multiple messages have this Message-ID (diff)
From: Joe Perches <joe@perches.com>
To: Jean Delvare <jdelvare@suse.de>
Cc: Julia Lawall <julia.lawall@lip6.fr>,
	Al Viro <viro@ZenIV.linux.org.uk>,
	Ilya Dryomov <idryomov@gmail.com>,
	Andy Whitcroft <apw@canonical.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Ceph Development <ceph-devel@vger.kernel.org>,
	Alex Elder <elder@kernel.org>, Sage Weil <sage@redhat.com>,
	LKML <linux-kernel@vger.kernel.org>,
	kernel-janitors@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-doc@vger.kernel.org
Subject: Re: "CodingStyle: Clarify and complete chapter 7" in docs-next
Date: Thu, 22 Sep 2016 10:42:10 +0000	[thread overview]
Message-ID: <1474540930.8253.9.camel@perches.com> (raw)
In-Reply-To: <20160922112407.47da9393@endymion>

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.
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2016-09-22 10:42 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-19 11:53 "CodingStyle: Clarify and complete chapter 7" in docs-next (was Re: [PATCH 03/47] block-rbd: Adjust the position of a jump label in rbd_header_from_disk()) Ilya Dryomov
2016-09-19 11:53 ` "CodingStyle: Clarify and complete chapter 7" in docs-next (was Re: [PATCH 03/47] block-rbd: Adjust Ilya Dryomov
2016-09-20  0:11 ` "CodingStyle: Clarify and complete chapter 7" in docs-next (was Re: [PATCH 03/47] block-rbd: Adjust the position of a jump label in rbd_header_from_disk()) Al Viro
2016-09-20  0:11   ` "CodingStyle: Clarify and complete chapter 7" in docs-next (was Re: [PATCH 03/47] block-rbd: Adj Al Viro
2016-09-20  2:46   ` "CodingStyle: Clarify and complete chapter 7" in docs-next (was Re: [PATCH 03/47] block-rbd: Adjust the position of a jump label in rbd_header_from_disk()) Joe Perches
2016-09-20  2:46     ` "CodingStyle: Clarify and complete chapter 7" in docs-next (was Re: [PATCH 03/47] block-rbd: Adj Joe Perches
2016-09-20  5:53     ` "CodingStyle: Clarify and complete chapter 7" in docs-next (was Re: [PATCH 03/47] block-rbd: Adjust the position of a jump label in rbd_header_from_disk()) Julia Lawall
2016-09-20  5:53       ` "CodingStyle: Clarify and complete chapter 7" in docs-next (was Re: [PATCH 03/47] block-rbd: Adj Julia Lawall
2016-09-20  6:32       ` "CodingStyle: Clarify and complete chapter 7" in docs-next (was Re: [PATCH 03/47] block-rbd: Adjust the position of a jump label in rbd_header_from_disk()) Joe Perches
2016-09-20  6:32         ` "CodingStyle: Clarify and complete chapter 7" in docs-next (was Re: [PATCH 03/47] block-rbd: Adj Joe Perches
2016-09-20  6:46         ` "CodingStyle: Clarify and complete chapter 7" in docs-next (was Re: [PATCH 03/47] block-rbd: Adjust the position of a jump label in rbd_header_from_disk()) Julia Lawall
2016-09-20  6:46           ` "CodingStyle: Clarify and complete chapter 7" in docs-next (was Re: [PATCH 03/47] block-rbd: Adj Julia Lawall
2016-09-22  9:24         ` "CodingStyle: Clarify and complete chapter 7" in docs-next Jean Delvare
2016-09-22  9:24           ` Jean Delvare
2016-09-22 10:42           ` Joe Perches [this message]
2016-09-22 10:42             ` Joe Perches
2016-09-22 11:57             ` Jean Delvare
2016-09-22 11:57               ` Jean Delvare
2016-09-22 13:11               ` Al Viro
2016-09-22 13:11                 ` Al Viro
2016-09-22 14:58                 ` Jean Delvare
2016-09-22 14:58                   ` Jean Delvare
2016-09-22 15:05                   ` Julia Lawall
2016-09-22 15:05                     ` Julia Lawall
2016-09-22 17:50                 ` Joe Perches
2016-09-22 17:50                   ` Joe Perches
2016-09-22 17:49               ` Joe Perches
2016-09-22 17:49                 ` Joe Perches
2016-09-22 19:47                 ` Jean Delvare
2016-09-22 19:47                   ` Jean Delvare
2016-09-22 10:43           ` Jani Nikula
2016-09-22 10:43             ` Jani Nikula
2016-09-22 12:46             ` Jean Delvare
2016-09-22 12:46               ` Jean Delvare
2016-09-22 13:06               ` Jani Nikula
2016-09-22 13:06                 ` Jani Nikula

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=1474540930.8253.9.camel@perches.com \
    --to=joe@perches.com \
    --cc=akpm@linux-foundation.org \
    --cc=apw@canonical.com \
    --cc=ceph-devel@vger.kernel.org \
    --cc=corbet@lwn.net \
    --cc=elder@kernel.org \
    --cc=idryomov@gmail.com \
    --cc=jdelvare@suse.de \
    --cc=julia.lawall@lip6.fr \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sage@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@ZenIV.linux.org.uk \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.