LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* Make checkpatch.pl's quiet option not print the summary on no errors
@ 2008-01-03  0:54 Arjan van de Ven
  2008-01-03 13:09 ` Andy Whitcroft
  2008-01-15 20:10 ` Andy Whitcroft
  0 siblings, 2 replies; 6+ messages in thread
From: Arjan van de Ven @ 2008-01-03  0:54 UTC (permalink / raw)
  To: apw; +Cc: linux-kernel

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";

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Make checkpatch.pl's quiet option not print the summary on no errors
  2008-01-03  0:54 Make checkpatch.pl's quiet option not print the summary on no errors Arjan van de Ven
@ 2008-01-03 13:09 ` Andy Whitcroft
  2008-01-15 20:10 ` Andy Whitcroft
  1 sibling, 0 replies; 6+ messages in thread
From: Andy Whitcroft @ 2008-01-03 13:09 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Make checkpatch.pl's quiet option not print the summary on no errors
  2008-01-03  0:54 Make checkpatch.pl's quiet option not print the summary on no errors Arjan van de Ven
  2008-01-03 13:09 ` Andy Whitcroft
@ 2008-01-15 20:10 ` Andy Whitcroft
  2008-01-15 21:58   ` Ingo Molnar
  2008-01-16 11:06   ` Ingo Molnar
  1 sibling, 2 replies; 6+ messages in thread
From: Andy Whitcroft @ 2008-01-15 20:10 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: linux-kernel, Ingo Molnar

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Make checkpatch.pl's quiet option not print the summary on no errors
  2008-01-15 20:10 ` Andy Whitcroft
@ 2008-01-15 21:58   ` Ingo Molnar
  2008-01-16 11:06   ` Ingo Molnar
  1 sibling, 0 replies; 6+ messages in thread
From: Ingo Molnar @ 2008-01-15 21:58 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: Arjan van de Ven, linux-kernel


* 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Make checkpatch.pl's quiet option not print the summary on no errors
  2008-01-15 20:10 ` Andy Whitcroft
  2008-01-15 21:58   ` Ingo Molnar
@ 2008-01-16 11:06   ` Ingo Molnar
  2008-01-17 17:05     ` Andy Whitcroft
  1 sibling, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2008-01-16 11:06 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: Arjan van de Ven, linux-kernel


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Make checkpatch.pl's quiet option not print the summary on no errors
  2008-01-16 11:06   ` Ingo Molnar
@ 2008-01-17 17:05     ` Andy Whitcroft
  0 siblings, 0 replies; 6+ messages in thread
From: Andy Whitcroft @ 2008-01-17 17:05 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Arjan van de Ven, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2008-01-17 17:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-01-03  0:54 Make checkpatch.pl's quiet option not print the summary on no errors Arjan van de Ven
2008-01-03 13:09 ` Andy Whitcroft
2008-01-15 20:10 ` Andy Whitcroft
2008-01-15 21:58   ` Ingo Molnar
2008-01-16 11:06   ` Ingo Molnar
2008-01-17 17:05     ` Andy Whitcroft

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).