From: Johan Hovold <johan@kernel.org>
To: Alex Elder <elder@linaro.org>
Cc: Greg KH <gregkh@linuxfoundation.org>,
Madhumitha Prabakaran <madhumithabiw@gmail.com>,
devel@driverdev.osuosl.org, greybus-dev@lists.linaro.org,
elder@kernel.org, johan@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [greybus-dev] [PATCH v2] Staging: greybus: Cleanup in greybus driver
Date: Wed, 17 Apr 2019 13:39:36 +0200 [thread overview]
Message-ID: <20190417113936.GB2520@localhost> (raw)
In-Reply-To: <a6bb72ed-5f98-86fd-33a3-8d68942eeae2@linaro.org>
On Wed, Apr 17, 2019 at 06:19:50AM -0500, Alex Elder wrote:
> I'm not completely sure about the inline function, but on the no blank
> lines thing (and many other minor issues) "checkpatch.pl" is to blame.
> There are lots of examples of issues that checkpatch points out that are
> matters of opinion and not hardened kernel style rules.
>
> We try to encourage people to get involved with kernel development by
> fixing minor problems, and we tell them a good way to find them is
> by running checkpatch and "fixing" what it reports. Unfortunately,
> it is often things of this type, and reviewers balk and say "no,
> please leave it," and the poor new person has a bad first experience.
>
> I *like* "checkpatch.pl". And the fact that it can point out some
> of these sorts of things is great. But it would be nice if certain
> types of problems (like multiple blank lines, or lines that are 81
> characters wide for example) would only be reported when a "--strict"
> option or something were supplied.
The problem is that --strict is enabled by default when running
checkpatch on code in staging (and net).
And it has all sorts of weird tests (prefixed as "CHECK" rather than
"WARNING") to catch everyone and their mom's pet peeve.
I don't think the intention ever was that all those should be "fixed",
but this appears to be where this checkpatch mission creep comes from
(and we're now seeing --strict being used on code outside of staging
too).
IMO we're setting a bad example for new contributers by accepting such
changes by default. Blindly trusting a tool is not how kernel
development works, but that seems to be the message currently sent.
Johan
next prev parent reply other threads:[~2019-04-17 11:39 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-16 22:13 [PATCH v2] Staging: greybus: Cleanup in greybus driver Madhumitha Prabakaran
2019-04-17 2:11 ` [greybus-dev] " Viresh Kumar
2019-04-17 6:25 ` Greg KH
2019-04-17 11:19 ` [greybus-dev] " Alex Elder
2019-04-17 11:39 ` Johan Hovold [this message]
2019-04-18 0:55 ` Madhumitha Prabakaran
2019-04-17 11:41 ` [greybus-dev] " Alex Elder
2019-04-18 0:58 ` Madhumitha Prabakaran
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=20190417113936.GB2520@localhost \
--to=johan@kernel.org \
--cc=devel@driverdev.osuosl.org \
--cc=elder@kernel.org \
--cc=elder@linaro.org \
--cc=gregkh@linuxfoundation.org \
--cc=greybus-dev@lists.linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=madhumithabiw@gmail.com \
/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