[PATCH] Deprecate checkpatch.pl --file mode; add warning; add --file-force

[PATCH] Deprecate checkpatch.pl --file mode; add warning; add --file-force

[Andi Kleen]

I'm not entirely sure who collects checkpatch patches for .25 -- i thought it
was Andy W., but he doesn't seem to answer. Anyways whoever does it please
queue this patch for the next checkpatch.pl update. Or perhaps it could
be put into -mm?

Thanks,
-Andi
----

Deprecate checkpatch.pl --file mode; add warning; add --file-force

As discussed on linux-kernel checkpatch.pl only patches for whole
files have a significant cost. Better such changes should be only
done together with other changes. Add a explicit warning about
this; deprecate --file and add a --file-force instead.

Signed-off-by: Andi Kleen <ak@suse.de>

---
 scripts/checkpatch.pl |   20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

Index: linux/scripts/checkpatch.pl
===================================================================
--- linux.orig/scripts/checkpatch.pl
+++ linux/scripts/checkpatch.pl
@@ -21,6 +21,7 @@ my $tst_type = 0;
 my $emacs = 0;
 my $terse = 0;
 my $file = 0;
+my $file_force = 0;
 my $check = 0;
 my $summary = 1;
 my $mailback = 0;
@@ -34,6 +35,7 @@ GetOptions(
 	'emacs!'	=> \$emacs,
 	'terse!'	=> \$terse,
 	'file!'		=> \$file,
+	'file-force!'	=> \$file_force,
 	'subjective!'	=> \$check,
 	'strict!'	=> \$check,
 	'root=s'	=> \$root,
@@ -50,12 +52,28 @@ if ($#ARGV < 0) {
 	print "         --no-tree    => run without a kernel tree\n";
 	print "         --terse      => one line per report\n";
 	print "         --emacs      => emacs compile window format\n";
-	print "         --file       => check a source file\n";
+	print " 	--file	     => check a source file (deprecated)\n";
+	print " 	--file-force => check a source file\n";
 	print "         --strict     => enable more subjective tests\n";
 	print "         --root       => path to the kernel tree root\n";
 	exit(1);
 }
 
+if ($file) {
+	print <<EOL
+WARNING: Using --file mode. Please do not send patches to linux-kernel
+that change whole existing files if you did not significantly change most
+of the the file for other reasons anyways or just wrote the file newly
+from scratch. Pure code style patches have a significant cost in a
+quickly changing code base like Linux because they cause rejects
+with other changes.
+If you're sure you want to use whole file mode please use --file-force
+EOL
+;
+	exit(1);
+};
+$file = $file_force;
+
 if ($terse) {
 	$emacs = 1;
 	$quiet++;

Re: [PATCH] Deprecate checkpatch.pl --file mode; add warning; add --file-force

[Andy Whitcroft]

On Wed, Jan 16, 2008 at 11:21:21PM +0100, Andi Kleen wrote:
> 
> I'm not entirely sure who collects checkpatch patches for .25 -- i thought it
> was Andy W., but he doesn't seem to answer. Anyways whoever does it please
> queue this patch for the next checkpatch.pl update. Or perhaps it could
> be put into -mm?

It is me, I have been off on holiday skiing.  I have a whole bunch of
updates merged already which I will send out tommorrow.  Your --file is
in the next batch which should be tested and ready by next week.

Sorry for the delay.

-apw

Re: [PATCH] Deprecate checkpatch.pl --file mode; add warning; add --file-force

[Andy Whitcroft]

On Wed, Jan 16, 2008 at 11:21:21PM +0100, Andi Kleen wrote:

> Deprecate checkpatch.pl --file mode; add warning; add --file-force
> 
> As discussed on linux-kernel checkpatch.pl only patches for whole
> files have a significant cost. Better such changes should be only
> done together with other changes. Add a explicit warning about
> this; deprecate --file and add a --file-force instead.
> 
> Signed-off-by: Andi Kleen <ak@suse.de>

Cirtainly I can see why we want this
warning out there and very much in the submitters face, it just feels a
little heavy handed to make them change option to get the result.

Would not just always appending this message in --file mode always be
just as effective?  Perhaps suppressing it with -q, or with some new "i
know what I am doing" option?

-apw