Messages in this thread | | | Date | Thu, 1 Feb 2018 09:03:48 -0700 | From | Jonathan Corbet <> | Subject | Re: [PATCH] checkpatch: warn if changed lines exceeds a maximum size |
| |
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
| |