public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Sarah Sharp <sarah.a.sharp@linux.intel.com>
To: Andy Whitcroft <apw@canonical.com>
Cc: linux-kernel@vger.kernel.org,
	James Bottomley <James.Bottomley@HansenPartnership.com>
Subject: [RFC v2] Checkpatch: add option to change warning return value.
Date: Tue, 22 Mar 2011 16:40:49 -0700	[thread overview]
Message-ID: <20110322234049.GA8083@xanatos> (raw)
In-Reply-To: <20110322232246.GA7450@xanatos>

There are a lot of warnings in checkpatch.pl that are rather subjective.
For instance, when a line is 81 characters long, and the patch submitter
is just cleaning up existing code, a maintainer may not care that
checkpatch.pl warns about the over 80-character line.  It should be up to
the maintainer what warnings they want to pay attention to.

Unfortunately, if you try to run checkpatch.pl as part of a git pre-commit
hook and a patch in a series being applied by git-am fails checkpatch.pl,
the whole process stops.  At that point I usually end up just disabling
that git hook, which means I don't see real errors in the patches.

Add an option to checkpatch.pl to print the warnings, but not return an
error code when the --lazy flag is present.  (Anyone with a better name
can pipe up, lazy just seemed to be the opposite to strict.)  This allows
the maintainer to view the warnings, but still allow the patch application
process to continue.

Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
---

Ok, I think this version works correctly.  My git hook returns an error
when the code to be submitted causes a checkpatch error, and doesn't
return an error code (but still prints the warnings) when the code to be
committed triggers checkpatch warnings, when --lazy is used.  This
version also changes the summary message to be unhappy when warnings are
found.

 scripts/checkpatch.pl |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4c0383d..ad2d423 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -15,6 +15,7 @@ my $V = '0.31';
 use Getopt::Long qw(:config no_auto_abbrev);
 
 my $quiet = 0;
+my $lazy = 0;
 my $tree = 1;
 my $chk_signoff = 1;
 my $chk_patch = 1;
@@ -46,6 +47,7 @@ Options:
   --terse                    one line per report
   -f, --file                 treat FILE as regular source file
   --subjective, --strict     enable more subjective tests
+  --lazy                     print warnings, but don't return an error condition
   --root=PATH                PATH to the kernel tree root
   --no-summary               suppress the per-file summary
   --mailback                 only produce a report in case of warnings/errors
@@ -80,6 +82,7 @@ GetOptions(
 
 	'debug=s'	=> \%debug,
 	'test-only=s'	=> \$tst_only,
+	'lazy+'		=> \$lazy,
 	'h|help'	=> \$help,
 	'version'	=> \$help
 ) or help(1);
@@ -1109,7 +1112,9 @@ sub ERROR {
 }
 sub WARN {
 	if (report("WARNING: $_[0]\n")) {
-		our $clean = 0;
+		if ($lazy == 0) {
+			our $clean = 0;
+		}
 		our $cnt_warn++;
 	}
 }
@@ -2952,10 +2957,10 @@ sub process {
 		}
 	}
 
-	if ($clean == 1 && $quiet == 0) {
+	if ($clean == 1 && $quiet == 0 && ($lazy == 0 || $cnt_warn == 0)) {
 		print "$vname has no obvious style problems and is ready for submission.\n"
 	}
-	if ($clean == 0 && $quiet == 0) {
+	if (($clean == 0 && $quiet == 0) || ($lazy == 1 && $cnt_warn != 0)) {
 		print "$vname has style problems, please review.  If any of these errors\n";
 		print "are false positives report them to the maintainer, see\n";
 		print "CHECKPATCH in MAINTAINERS.\n";
-- 
1.7.1


  reply	other threads:[~2011-03-22 23:40 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-22 23:13 [RFC] checkpatch: add option to change warning return value Sarah Sharp
2011-03-22 23:22 ` Sarah Sharp
2011-03-22 23:40   ` Sarah Sharp [this message]
2011-03-23  1:41   ` Steven Rostedt
2011-03-24 15:40     ` Sarah Sharp
2011-03-24 18:23       ` Steven Rostedt
2011-03-24 22:02         ` Sarah Sharp
2011-03-23 10:07 ` Johannes Weiner
2011-03-23 18:43   ` Sarah Sharp

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=20110322234049.GA8083@xanatos \
    --to=sarah.a.sharp@linux.intel.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=apw@canonical.com \
    --cc=linux-kernel@vger.kernel.org \
    /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