Subject: Make checkpatch.pl's quiet option not print the summary on no errors
From: Arjan van de Ven <[email protected]>
CC: [email protected]
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 <[email protected]>
---
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";
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 <[email protected]>
> CC: [email protected]
>
> 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 <[email protected]>
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
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 <[email protected]>
> CC: [email protected]
>
> 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 <[email protected]>
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
* Andy Whitcroft <[email protected]> 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 <[email protected]>
> > CC: [email protected]
> >
> > 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 <[email protected]>
>
> 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
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
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