public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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

  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