From: Jonathan Corbet <corbet@lwn.net>
To: "Brown, Nicholas" <nb930b@intl.att.com>
Cc: "joe@perches.com" <joe@perches.com>,
"apw@canonical.com" <apw@canonical.com>,
"me@tobin.cc" <me@tobin.cc>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] checkpatch: warn if changed lines exceeds a maximum size
Date: Thu, 1 Feb 2018 09:03:48 -0700 [thread overview]
Message-ID: <20180201090348.61c9be3f@lwn.net> (raw)
In-Reply-To: <1517499752.3668.10.camel@intl.att.com>
On Thu, 1 Feb 2018 15:42:35 +0000
"Brown, Nicholas" <nb930b@intl.att.com> wrote:
> > I think the metric is too simplistic and
> > not particularly useful.
>
> I'm not sure it's any more simplistic than than the character line
> length limit, which is there to prompt thought on code nesting levels,
> etc. And as it has to be explicitly configured it allows developers the
> discretion to determine a code change size that meaningful in a given
> situation.
The line-length limit relates directly to code readability and
non-infringement of a developer's sacred right to work unimpeded on an
80x24 xterm (the last vt100 died, unfortunately).
A line-count limit lacks even that justification. The rule on splitting
patches is entirely about logical changes that can be reviewed
independently. Some of those changes involve a lot of lines, others do
not. If the correct splits do not come to you when you're writing the
changelogs for your patches (or before), a tool nagging about line counts
really isn't going to help. I would expect it to miss most patches
actually in need of splitting while complaining about many patches that
are just fine.
Thanks for working to improve the tools - they certainly can afford a lot
of improvement! But my own suggestion would be to look a bit further for
improvements that will be truly helpful to the development community.
Thanks,
jon
next prev parent reply other threads:[~2018-02-01 16:03 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-30 15:57 [PATCH] checkpatch: warn if changed lines exceeds a maximum size Brown, Nicholas
2018-01-30 16:10 ` Joe Perches
2018-01-30 18:04 ` Brown, Nicholas
2018-01-30 19:01 ` Nicholas Brown
2018-01-30 19:09 ` Joe Perches
2018-01-30 20:25 ` Brown, Nicholas
2018-01-30 20:26 ` Nicholas Brown
[not found] ` <1517481518.3063.92.camel@intl.att.com>
2018-02-01 12:54 ` Joe Perches
2018-02-01 15:42 ` Brown, Nicholas
2018-02-01 16:03 ` Jonathan Corbet [this message]
-- strict thread matches above, loose matches on Subject: below --
2018-01-30 15:04 Brown, Nicholas
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=20180201090348.61c9be3f@lwn.net \
--to=corbet@lwn.net \
--cc=apw@canonical.com \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
--cc=me@tobin.cc \
--cc=nb930b@intl.att.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