Make checkpatch.pl's quiet option not print the summary on no errors

Make checkpatch.pl's quiet option not print the summary on no errors

[Arjan van de Ven]

Subject: Make checkpatch.pl's quiet option not print the summary on no errors
From: Arjan van de Ven <arjan@linux.intel.com>
CC: apw@uk.ibm.com

Right now, in quiet mode, checkpatch.pl still prints a summary line even
if the patch is 100% clean. IMO, "quiet mode" should mean "no output if clean",
the patch below makes that so. (This also makes the quilt integration
on my system work nicer :)

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

---
  scripts/checkpatch.pl |    2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.24-rc6/scripts/checkpatch.pl
===================================================================
--- linux-2.6.24-rc6.orig/scripts/checkpatch.pl
+++ linux-2.6.24-rc6/scripts/checkpatch.pl
@@ -1579,7 +1579,7 @@ sub process {
  	}

  	print report_dump();
-	if ($summary) {
+	if ($summary && ($clean == 0 || $quiet == 0)) {
  		print "total: $cnt_error errors, $cnt_warn warnings, " .
  			(($check)? "$cnt_chk checks, " : "") .
  			"$cnt_lines lines checked\n";

Re: Make checkpatch.pl's quiet option not print the summary on no errors

[Andy Whitcroft]

On Thu, Jan 03, 2008 at 01:54:42AM +0100, Arjan van de Ven wrote:
> Subject: Make checkpatch.pl's quiet option not print the summary on no 
> errors
> From: Arjan van de Ven <arjan@linux.intel.com>
> CC: apw@uk.ibm.com
> 
> Right now, in quiet mode, checkpatch.pl still prints a summary line even
> if the patch is 100% clean. IMO, "quiet mode" should mean "no output if 
> clean",
> the patch below makes that so. (This also makes the quilt integration
> on my system work nicer :)
> 
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

I think that sounds reasonable.  Should people want the summary
regardless it makes more sense to provice an option for that, perhaps a
doubling of the --summary flag.

Will be in 0.13.

-apw

Re: Make checkpatch.pl's quiet option not print the summary on no errors

[Andy Whitcroft]

On Thu, Jan 03, 2008 at 01:54:42AM +0100, Arjan van de Ven wrote:
> Subject: Make checkpatch.pl's quiet option not print the summary on no 
> errors
> From: Arjan van de Ven <arjan@linux.intel.com>
> CC: apw@uk.ibm.com
> 
> Right now, in quiet mode, checkpatch.pl still prints a summary line even
> if the patch is 100% clean. IMO, "quiet mode" should mean "no output if 
> clean",
> the patch below makes that so. (This also makes the quilt integration
> on my system work nicer :)
> 
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

While looking to integrate this I discovered that the current default
was a desired feature requested by Ingo.  So I guess we need to come up
with a combination of options which give us both.

Currently we have --[no-]summary meaning suppress/add a summary, and
--quiet meaning suppress output but which does not suppress the summary.

We have a few options:

1) allow doubling of -q to make the summary subject to -q,
2) allow doubling of --summary to mean "override -q", --summary becoming
   subject to -q,
3) add a new option --force-summary which always produces a summary,
   --summary becoming subject to -q, and
4) add a new option --summary-on-fail which is subject to -q.

I feel the last of these is the most obvious option, and carries
no modification to current semantics.

Thoughts?

-apw

Re: Make checkpatch.pl's quiet option not print the summary on no errors

[Ingo Molnar]

* Andy Whitcroft <andyw@uk.ibm.com> wrote:

> On Thu, Jan 03, 2008 at 01:54:42AM +0100, Arjan van de Ven wrote:
> > Subject: Make checkpatch.pl's quiet option not print the summary on no 
> > errors
> > From: Arjan van de Ven <arjan@linux.intel.com>
> > CC: apw@uk.ibm.com
> > 
> > Right now, in quiet mode, checkpatch.pl still prints a summary line 
> > even if the patch is 100% clean. IMO, "quiet mode" should mean "no 
> > output if clean", the patch below makes that so. (This also makes 
> > the quilt integration on my system work nicer :)
> > 
> > Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> 
> While looking to integrate this I discovered that the current default 
> was a desired feature requested by Ingo.  So I guess we need to come 
> up with a combination of options which give us both.

feel free to drop the summary line on -q, i think i only wanted the 
summary to be avilable _somewhere_, so that there's a "quality metric at 
glance" line that one can see. To me that still plays OK with quilt 
because it's a constant reminder that the patch is clean.

	Ingo

Re: Make checkpatch.pl's quiet option not print the summary on no errors

[Ingo Molnar]

btw, just found a checkpatch.pl buglet, it gets confused on zero-sized 
files:

 $ echo -n > /tmp/1.c
 $ scripts/checkpatch.pl --file /tmp/1.c
 ERROR: Does not appear to be a unified-diff format patch

 total: 1 errors, 0 warnings, 0 lines checked

 Your patch has style problems, please review.  If any of these errors
 are false positives report them to the maintainer, see
 CHECKPATCH in MAINTAINERS.

(this broke my code-quality scriptlet)

	Ingo

Re: Make checkpatch.pl's quiet option not print the summary on no errors

[Andy Whitcroft]

On Wed, Jan 16, 2008 at 12:06:46PM +0100, Ingo Molnar wrote:
> 
> btw, just found a checkpatch.pl buglet, it gets confused on zero-sized 
> files:
> 
>  $ echo -n > /tmp/1.c
>  $ scripts/checkpatch.pl --file /tmp/1.c
>  ERROR: Does not appear to be a unified-diff format patch
> 
>  total: 1 errors, 0 warnings, 0 lines checked
> 
>  Your patch has style problems, please review.  If any of these errors
>  are false positives report them to the maintainer, see
>  CHECKPATCH in MAINTAINERS.
> 
> (this broke my code-quality scriptlet)

Sigh ... :)

Thanks I'll sort that out.

-apw