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: Re: [RFC] checkpatch: add option to change warning return value.
Date: Tue, 22 Mar 2011 16:22:46 -0700 [thread overview]
Message-ID: <20110322232246.GA7450@xanatos> (raw)
In-Reply-To: <20110322231310.GA7338@xanatos>
On Tue, Mar 22, 2011 at 04:13:10PM -0700, Sarah Sharp wrote:
> 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.
BTW, I'm not a perl hacker, and I don't really understand the reporting code,
so someone who knows it (Andy?) should make the correct patch. I didn't notice
before sending, but right now the warning count reporting is bogus, although
the warnings don't make the script return an error code:
sarah@xanatos:~/git/kernels/xhci$ git am ~/Maildir.fetchmail/.to-apply/
Applying: USB: xhci - fix unsafe macro definitions
total: 0 errors, 0 warnings, 21 lines checked
Your patch has no obvious style problems and is ready for submission.
Applying: USB: xhci: unsigned char never equals -1
total: 0 errors, 0 warnings, 55 lines checked
Your patch has no obvious style problems and is ready for submission.
Applying: USB: xhci: simplify logic of skipping missed isoc TDs
WARNING: line over 80 characters
#12: FILE: drivers/usb/host/xhci-ring.c:1669:
+ struct xhci_ring *ep_ring = xhci_dma_to_transfer_ring(ep, event->buffer);
WARNING: line over 80 characters
#136: FILE: drivers/usb/host/xhci-ring.c:1743:
+ struct xhci_ring *ep_ring = xhci_dma_to_transfer_ring(ep, event->buffer);
WARNING: line over 80 characters
#164: FILE: drivers/usb/host/xhci-ring.c:1770:
+ struct xhci_ring *ep_ring = xhci_dma_to_transfer_ring(ep, event->buffer);
total: 0 errors, 0 warnings, 231 lines checked
Your patch has no obvious style problems and is ready for submission.
Applying: USB: fix formatting of SuperSpeed endpoints in /proc/bus/usb/devices
total: 0 errors, 0 warnings, 28 lines checked
Your patch has no obvious style problems and is ready for submission.
Note that second-to-last patch should have said 3 warnings. So my patch needs
to be changed, but I really do want this functionality.
Sarah Sharp
> Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
> ---
> scripts/checkpatch.pl | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 4c0383d..4519b5b 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);
> @@ -1096,6 +1099,9 @@ sub report {
>
> push(our @report, $line);
>
> + if ($lazy == 1) {
> + return 0;
> + }
> return 1;
> }
> sub report_dump {
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
next prev parent reply other threads:[~2011-03-22 23:22 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 [this message]
2011-03-22 23:40 ` [RFC v2] Checkpatch: " Sarah Sharp
2011-03-23 1:41 ` [RFC] checkpatch: " 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=20110322232246.GA7450@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.