Re: [PATCH] checkpatch: warn if changed lines exceeds a maximum size

[Joe Perches <joe@perches.com>]

On Thu, 2018-02-01 at 10:38 +0000, Brown, Nicholas wrote:
> Hi,
> 
> Would one of you be happy to pick this patch up for carrying into the next release?

Not me.

I think the metric is too simplistic and
not particularly useful.

If others want it, I think you should
try to build or find a bit more agreement
from the many other linux developers that
this change is actually desired by them.

> Thanks,
> Nick

cheers, Joe

> 
> On Tue, 2018-01-30 at 20:26 +0000, Nicholas Brown wrote:
> > Changed lines is the total of inserted and deleted lines.
> > By default there is no limit, --max-changed-lines may be used to set a
> > value. Some users may wish to encourage that patches are split into
> > smaller parts using this.
> > See Documentation/process/submitting-patches.rst#split-changes
> > 
> > Signed-off-by: Nicholas Brown <nick.brown@att.com>
> > ---
> >  scripts/checkpatch.pl | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> > 
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 31031f10fe56..a71bc4f15ce7 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -49,6 +49,7 @@ my @ignore = ();
> >  my $help = 0;
> >  my $configuration_file = ".checkpatch.conf";
> >  my $max_line_length = 80;
> > +my $max_changed_lines; # undef = no max
> >  my $ignore_perl_version = 0;
> >  my $minimum_perl_version = 5.10.0;
> >  my $min_conf_desc_length = 4;
> > @@ -92,6 +93,8 @@ Options:
> >    --ignore TYPE(,TYPE2...)   ignore various comma separated message types
> >    --show-types               show the specific message type in the output
> >    --max-line-length=n        set the maximum line length, if exceeded, warn
> > +  --max-changed-lines=n      set the maximum number of changed lines allowed,
> > +                             if exceeded, warn. (insertions + deletions)
> >    --min-conf-desc-length=n   set the min description length, if shorter, warn
> >    --root=PATH                PATH to the kernel tree root
> >    --no-summary               suppress the per-file summary
> > @@ -209,6 +212,7 @@ GetOptions(
> >  	'show-types!'	=> \$show_types,
> >  	'list-types!'	=> \$list_types,
> >  	'max-line-length=i' => \$max_line_length,
> > +	'max-changed-lines=i' => \$max_changed_lines,
> >  	'min-conf-desc-length=i' => \$min_conf_desc_length,
> >  	'root=s'	=> \$root,
> >  	'summary!'	=> \$summary,
> > @@ -2165,6 +2169,8 @@ sub process {
> >  	my $filename = shift;
> >  
> >  	my $linenr=0;
> > +	my $inserted_lines_total=0;
> > +	my $deleted_lines_total=0;
> >  	my $prevline="";
> >  	my $prevrawline="";
> >  	my $stashline="";
> > @@ -2233,6 +2239,14 @@ sub process {
> >  
> >  		push(@fixed, $rawline) if ($fix);
> >  
> > +		if ($rawline=~/^\+/ && $rawline!~/^\+\+\+/) {
> > +			$inserted_lines_total++
> > +		}
> > +
> > +		if ($rawline=~/^-/ && $rawline!~/^---/) {
> > +			$deleted_lines_total++;
> > +		}
> > +
> >  		if ($rawline=~/^\+\+\+\s+(\S+)/) {
> >  			$setup_docs = 0;
> >  			if ($1 =~ m@Documentation/admin-guide/kernel-parameters.rst$@) {
> > @@ -2306,6 +2320,13 @@ sub process {
> >  
> >  	$prefix = '';
> >  
> > +	#print "inserted: $inserted_lines_total\n";
> > +	#print "deleted: $deleted_lines_total\n";

Not much utility in the debugging code either.

> > +	if (defined $max_changed_lines &&
> > +	    ($inserted_lines_total+$deleted_lines_total > $max_changed_lines)) {
> > +		WARN("MAX_CHANGED_LINES", "please split the change into smaller parts\n");
> > +	}
> > +
> >  	$realcnt = 0;
> >  	$linenr = 0;
> >  	$fixlinenr = -1;