[PATCH] checkpatch: use patch subject when reading from stdin

[PATCH] checkpatch: use patch subject when reading from stdin

[Geert Uytterhoeven]

When reading a patch file from standard input, checkpatch calls it "Your
patch", and reports its state as:

    Your patch has style problems, please review.

or:

    Your patch has no obvious style problems and is ready for submission.

Hence when checking multiple patches by piping them to checkpatch, e.g.
when checking patchwork bundles using:

    formail -s scripts/checkpatch.pl < bundle-foo.mbox

it is difficult to identify which patches need to be reviewed and
improved.

Fix this by replacing "Your patch" by the patch subject, if present.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 scripts/checkpatch.pl | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 6fcc66afb0880830..6b9feb4d646a116b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1047,6 +1047,10 @@ for my $filename (@ARGV) {
 	}
 	while (<$FILE>) {
 		chomp;
+		if ($vname eq 'Your patch') {
+			my ($subject) = $_ =~ /^Subject:\s*(.*)/;
+			$vname = '"' . $subject . '"' if $subject;
+		}
 		push(@rawlines, $_);
 	}
 	close($FILE);
-- 
2.17.1

Re: [PATCH] checkpatch: use patch subject when reading from stdin

[Joe Perches]

On Tue, 2019-10-08 at 11:40 +0200, Geert Uytterhoeven wrote:
> When reading a patch file from standard input, checkpatch calls it "Your
> patch", and reports its state as:
> 
>     Your patch has style problems, please review.
> 
> or:
> 
>     Your patch has no obvious style problems and is ready for submission.
> 
> Hence when checking multiple patches by piping them to checkpatch, e.g.
> when checking patchwork bundles using:
> 
>     formail -s scripts/checkpatch.pl < bundle-foo.mbox
> 
> it is difficult to identify which patches need to be reviewed and
> improved.
> 
> Fix this by replacing "Your patch" by the patch subject, if present.

Seems sensible, thanks Geert

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  scripts/checkpatch.pl | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 6fcc66afb0880830..6b9feb4d646a116b 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1047,6 +1047,10 @@ for my $filename (@ARGV) {
>  	}
>  	while (<$FILE>) {
>  		chomp;
> +		if ($vname eq 'Your patch') {
> +			my ($subject) = $_ =~ /^Subject:\s*(.*)/;
> +			$vname = '"' . $subject . '"' if $subject;

trivia:

Not a big deal and is likely good enough but this will
cut off subjects that are continued on multiple lines.

e.g.:

Subject: [PATCH Vx n/M] very long description with a subject spanning
 multiple lines
From: patch submitter <submitter@domain.tld>

> +		}
>  		push(@rawlines, $_);
>  	}
>  	close($FILE);

Re: [PATCH] checkpatch: use patch subject when reading from stdin

[Geert Uytterhoeven]

Hi Joe,

On Tue, Oct 8, 2019 at 2:50 PM Joe Perches <joe@perches.com> wrote:
> On Tue, 2019-10-08 at 11:40 +0200, Geert Uytterhoeven wrote:
> > When reading a patch file from standard input, checkpatch calls it "Your
> > patch", and reports its state as:
> >
> >     Your patch has style problems, please review.
> >
> > or:
> >
> >     Your patch has no obvious style problems and is ready for submission.
> >
> > Hence when checking multiple patches by piping them to checkpatch, e.g.
> > when checking patchwork bundles using:
> >
> >     formail -s scripts/checkpatch.pl < bundle-foo.mbox
> >
> > it is difficult to identify which patches need to be reviewed and
> > improved.
> >
> > Fix this by replacing "Your patch" by the patch subject, if present.
>
> Seems sensible, thanks Geert
>
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> >  scripts/checkpatch.pl | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 6fcc66afb0880830..6b9feb4d646a116b 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -1047,6 +1047,10 @@ for my $filename (@ARGV) {
> >       }
> >       while (<$FILE>) {
> >               chomp;
> > +             if ($vname eq 'Your patch') {
> > +                     my ($subject) = $_ =~ /^Subject:\s*(.*)/;
> > +                     $vname = '"' . $subject . '"' if $subject;
>
> trivia:
>
> Not a big deal and is likely good enough but this will
> cut off subjects that are continued on multiple lines.
>
> e.g.:
>
> Subject: [PATCH Vx n/M] very long description with a subject spanning
>  multiple lines
> From: patch submitter <submitter@domain.tld>

I know.

Fixing that is not that simple, I'm afraid.
And $vname is used before process() is called.

>
> > +             }
> >               push(@rawlines, $_);
> >       }
> >       close($FILE);
>


-- 
Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] checkpatch: use patch subject when reading from stdin

[Joe Perches]

On Tue, 2019-10-08 at 11:40 +0200, Geert Uytterhoeven wrote:
> When reading a patch file from standard input, checkpatch calls it "Your
> patch", and reports its state as:
> 
>     Your patch has style problems, please review.
> 
> or:
> 
>     Your patch has no obvious style problems and is ready for submission.
> 
> Hence when checking multiple patches by piping them to checkpatch, e.g.
> when checking patchwork bundles using:
> 
>     formail -s scripts/checkpatch.pl < bundle-foo.mbox
> 
> it is difficult to identify which patches need to be reviewed and
> improved.
> 
> Fix this by replacing "Your patch" by the patch subject, if present.
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -1047,6 +1047,10 @@ for my $filename (@ARGV) {
>  	}
>  	while (<$FILE>) {
>  		chomp;
> +		if ($vname eq 'Your patch') {
> +			my ($subject) = $_ =~ /^Subject:\s*(.*)/;
> +			$vname = '"' . $subject . '"' if $subject;

Hi again Geert.

Just some stylistic nits:

$filename is not quoted so I think adding quotes
before and after $subject may not be useful.

Can you please use what checkpatch uses as a more
common parenthesis style after an if?

i.e. use:
	if (foo)
not
	if foo

so maybe:

	if ($filename eq '-' && $_ =~ /^Subject:\s*(.*)/) {
		$vname = $1;
	}

or maybe

	$vname = $1 if ($filename eq '-' && $_ =~ /^Subject:\s*(.*)/);

Re: [PATCH] checkpatch: use patch subject when reading from stdin

[Geert Uytterhoeven]

Hi Joe,

On Tue, Oct 8, 2019 at 5:20 PM Joe Perches <joe@perches.com> wrote:
> On Tue, 2019-10-08 at 11:40 +0200, Geert Uytterhoeven wrote:
> > When reading a patch file from standard input, checkpatch calls it "Your
> > patch", and reports its state as:
> >
> >     Your patch has style problems, please review.
> >
> > or:
> >
> >     Your patch has no obvious style problems and is ready for submission.
> >
> > Hence when checking multiple patches by piping them to checkpatch, e.g.
> > when checking patchwork bundles using:
> >
> >     formail -s scripts/checkpatch.pl < bundle-foo.mbox
> >
> > it is difficult to identify which patches need to be reviewed and
> > improved.
> >
> > Fix this by replacing "Your patch" by the patch subject, if present.
> []
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -1047,6 +1047,10 @@ for my $filename (@ARGV) {
> >       }
> >       while (<$FILE>) {
> >               chomp;
> > +             if ($vname eq 'Your patch') {
> > +                     my ($subject) = $_ =~ /^Subject:\s*(.*)/;
> > +                     $vname = '"' . $subject . '"' if $subject;
>
> Hi again Geert.
>
> Just some stylistic nits:
>
> $filename is not quoted so I think adding quotes
> before and after $subject may not be useful.

Filename is indeed not quoted, but $git_commits{$filename} is.

> Can you please use what checkpatch uses as a more
> common parenthesis style after an if?
>
> i.e. use:
>         if (foo)
> not
>         if foo
>
> so maybe:
>
>         if ($filename eq '-' && $_ =~ /^Subject:\s*(.*)/) {
>                 $vname = $1;
>         }
>
> or maybe
>
>         $vname = $1 if ($filename eq '-' && $_ =~ /^Subject:\s*(.*)/);

Thanks, will give it a try...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] checkpatch: use patch subject when reading from stdin

[Joe Perches]

On Tue, 2019-10-08 at 17:28 +0200, Geert Uytterhoeven wrote:
> Hi Joe,
> 
> On Tue, Oct 8, 2019 at 5:20 PM Joe Perches <joe@perches.com> wrote:
> > On Tue, 2019-10-08 at 11:40 +0200, Geert Uytterhoeven wrote:
> > > When reading a patch file from standard input, checkpatch calls it "Your
> > > patch", and reports its state as:
> > > 
> > >     Your patch has style problems, please review.
> > > 
> > > or:
> > > 
> > >     Your patch has no obvious style problems and is ready for submission.
> > > 
> > > Hence when checking multiple patches by piping them to checkpatch, e.g.
> > > when checking patchwork bundles using:
> > > 
> > >     formail -s scripts/checkpatch.pl < bundle-foo.mbox
> > > 
> > > it is difficult to identify which patches need to be reviewed and
> > > improved.
> > > 
> > > Fix this by replacing "Your patch" by the patch subject, if present.
> > []
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > []
> > > @@ -1047,6 +1047,10 @@ for my $filename (@ARGV) {
> > >       }
> > >       while (<$FILE>) {
> > >               chomp;
> > > +             if ($vname eq 'Your patch') {
> > > +                     my ($subject) = $_ =~ /^Subject:\s*(.*)/;
> > > +                     $vname = '"' . $subject . '"' if $subject;
> > 
> > Hi again Geert.
> > 
> > Just some stylistic nits:
> > 
> > $filename is not quoted so I think adding quotes
> > before and after $subject may not be useful.
> 
> Filename is indeed not quoted, but $git_commits{$filename} is.

If I understand your use case, this will only show the last
patch $subject of a bundle?

Also, it'll show things like "duplicate signature" when multiple
patches are tested in a single bundle.

For instance, if I have a git format-patch series in an output
directory and do

$ cat <output_dir>/*.patch | ./scripts/checkpatch.pl

Bad output happen.

Maybe this might be better:

---
 scripts/checkpatch.pl | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index cf7543a9d1b2..2f79c585e795 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2444,6 +2444,15 @@ sub process {
 
 		my $rawline = $rawlines[$linenr - 1];
 
+# if input from stdin, report the subject lines if they exist
+		if ($filename eq '-' && !$quiet &&
+		    $rawline =~ /^Subject:\s*(.*)/) {
+			report("stdin", "STDIN", '-' x length($1));
+			report("stdin", "STDIN", $1);
+			report("stdin", "STDIN", '-' x length($1));
+			%signatures = ();	# avoid duplicate signatures
+		}
+
 # check if it's a mode change, rename or start of a patch
 		if (!$in_commit_log &&
 		    ($line =~ /^ mode change [0-7]+ => [0-7]+ \S+\s*$/ ||

Re: [PATCH] checkpatch: use patch subject when reading from stdin

[Geert Uytterhoeven]

Hi Joe,

On Tue, Oct 8, 2019 at 7:02 PM Joe Perches <joe@perches.com> wrote:
> On Tue, 2019-10-08 at 17:28 +0200, Geert Uytterhoeven wrote:
> > On Tue, Oct 8, 2019 at 5:20 PM Joe Perches <joe@perches.com> wrote:
> > > On Tue, 2019-10-08 at 11:40 +0200, Geert Uytterhoeven wrote:
> > > > When reading a patch file from standard input, checkpatch calls it "Your
> > > > patch", and reports its state as:
> > > >
> > > >     Your patch has style problems, please review.
> > > >
> > > > or:
> > > >
> > > >     Your patch has no obvious style problems and is ready for submission.
> > > >
> > > > Hence when checking multiple patches by piping them to checkpatch, e.g.
> > > > when checking patchwork bundles using:
> > > >
> > > >     formail -s scripts/checkpatch.pl < bundle-foo.mbox
> > > >
> > > > it is difficult to identify which patches need to be reviewed and
> > > > improved.
> > > >
> > > > Fix this by replacing "Your patch" by the patch subject, if present.
> > > []
> > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > []
> > > > @@ -1047,6 +1047,10 @@ for my $filename (@ARGV) {
> > > >       }
> > > >       while (<$FILE>) {
> > > >               chomp;
> > > > +             if ($vname eq 'Your patch') {
> > > > +                     my ($subject) = $_ =~ /^Subject:\s*(.*)/;
> > > > +                     $vname = '"' . $subject . '"' if $subject;
> > >
> > > Hi again Geert.
> > >
> > > Just some stylistic nits:
> > >
> > > $filename is not quoted so I think adding quotes
> > > before and after $subject may not be useful.
> >
> > Filename is indeed not quoted, but $git_commits{$filename} is.
>
> If I understand your use case, this will only show the last
> patch $subject of a bundle?

False.
"formail -s scripts/checkpatch.pl < bundle-foo.mbox" splits
"bundle-foo.mbox" in separate patches, and invokes
"scripts/checkpatch.pl" for each of them.

> Also, it'll show things like "duplicate signature" when multiple
> patches are tested in a single bundle.

False, due to the splitting by formail.

> For instance, if I have a git format-patch series in an output
> directory and do
>
> $ cat <output_dir>/*.patch | ./scripts/checkpatch.pl
>
> Bad output happen.

Yeah, because you're concatenating all patches.
Currently it works for single patches only.

> Maybe this might be better:

> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2444,6 +2444,15 @@ sub process {
>
>                 my $rawline = $rawlines[$linenr - 1];
>
> +# if input from stdin, report the subject lines if they exist
> +               if ($filename eq '-' && !$quiet &&
> +                   $rawline =~ /^Subject:\s*(.*)/) {
> +                       report("stdin", "STDIN", '-' x length($1));
> +                       report("stdin", "STDIN", $1);
> +                       report("stdin", "STDIN", '-' x length($1));
> +                       %signatures = ();       # avoid duplicate signatures
> +               }
> +
>  # check if it's a mode change, rename or start of a patch
>                 if (!$in_commit_log &&
>                     ($line =~ /^ mode change [0-7]+ => [0-7]+ \S+\s*$/ ||

Perhaps.  Just passing the patchwork bundle to checkpatch, and fixing
checkpatch to handle multiple patches in a single file was my first idea.
But it looked fragile, with too much state that needs to be reset.
I.e. the state is not limited to %signatures.  You also have to reset
$author inside process(), and probably a dozen other variables.
And make sure that future changes don't forget resetting all newly
introduced variables.

Hence I settled for the solution using formail.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] checkpatch: use patch subject when reading from stdin

[Joe Perches]

On Tue, 2019-10-08 at 20:10 +0200, Geert Uytterhoeven wrote:
> Hi Joe,
> 
> On Tue, Oct 8, 2019 at 7:02 PM Joe Perches <joe@perches.com> wrote:
> > On Tue, 2019-10-08 at 17:28 +0200, Geert Uytterhoeven wrote:
> > > On Tue, Oct 8, 2019 at 5:20 PM Joe Perches <joe@perches.com> wrote:
> > > > On Tue, 2019-10-08 at 11:40 +0200, Geert Uytterhoeven wrote:
> > > > > When reading a patch file from standard input, checkpatch calls it "Your
> > > > > patch", and reports its state as:
> > > > > 
> > > > >     Your patch has style problems, please review.
> > > > > 
> > > > > or:
> > > > > 
> > > > >     Your patch has no obvious style problems and is ready for submission.
> > > > > 
> > > > > Hence when checking multiple patches by piping them to checkpatch, e.g.
> > > > > when checking patchwork bundles using:
> > > > > 
> > > > >     formail -s scripts/checkpatch.pl < bundle-foo.mbox
> > > > > 
> > > > > it is difficult to identify which patches need to be reviewed and
> > > > > improved.
> > > > > 
> > > > > Fix this by replacing "Your patch" by the patch subject, if present.
> > > > []
> > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > > []
> > > > > @@ -1047,6 +1047,10 @@ for my $filename (@ARGV) {
> > > > >       }
> > > > >       while (<$FILE>) {
> > > > >               chomp;
> > > > > +             if ($vname eq 'Your patch') {
> > > > > +                     my ($subject) = $_ =~ /^Subject:\s*(.*)/;
> > > > > +                     $vname = '"' . $subject . '"' if $subject;
> > > > 
> > > > Hi again Geert.
> > > > 
> > > > Just some stylistic nits:
> > > > 
> > > > $filename is not quoted so I think adding quotes
> > > > before and after $subject may not be useful.
> > > 
> > > Filename is indeed not quoted, but $git_commits{$filename} is.
> > 
> > If I understand your use case, this will only show the last
> > patch $subject of a bundle?
> 
> False.

Not really false, it's true.  Your use case just
doesn't submit bundled patches as a single input
to checkpatch.

> "formail -s scripts/checkpatch.pl < bundle-foo.mbox" splits
> "bundle-foo.mbox" in separate patches, and invokes
> "scripts/checkpatch.pl" for each of them.

Never used formail, it seems it was last updated in 2001.

> > Also, it'll show things like "duplicate signature" when multiple
> > patches are tested in a single bundle.
> 
> False, due to the splitting by formail.

Again, not false, just not your use.

> > For instance, if I have a git format-patch series in an output
> > directory and do
> > 
> > $ cat <output_dir>/*.patch | ./scripts/checkpatch.pl
> > 
> > Bad output happen.
> 
> Yeah, because you're concatenating all patches.
> Currently it works for single patches only.
> 
> > Maybe this might be better:
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -2444,6 +2444,15 @@ sub process {
> > 
> >                 my $rawline = $rawlines[$linenr - 1];
> > 
> > +# if input from stdin, report the subject lines if they exist
> > +               if ($filename eq '-' && !$quiet &&
> > +                   $rawline =~ /^Subject:\s*(.*)/) {
> > +                       report("stdin", "STDIN", '-' x length($1));
> > +                       report("stdin", "STDIN", $1);
> > +                       report("stdin", "STDIN", '-' x length($1));
> > +                       %signatures = ();       # avoid duplicate signatures
> > +               }
> > +
> >  # check if it's a mode change, rename or start of a patch
> >                 if (!$in_commit_log &&
> >                     ($line =~ /^ mode change [0-7]+ => [0-7]+ \S+\s*$/ ||
> 
> Perhaps.  Just passing the patchwork bundle to checkpatch, and fixing
> checkpatch to handle multiple patches in a single file was my first idea.
> But it looked fragile, with too much state that needs to be reset.
> I.e. the state is not limited to %signatures.  You also have to reset
> $author inside process(), and probably a dozen other variables.
> And make sure that future changes don't forget resetting all newly
> introduced variables.
> 
> Hence I settled for the solution using formail.

I still think the patch I suggested is better as it
functions for other use cases too.

Re: [PATCH] checkpatch: use patch subject when reading from stdin

[Geert Uytterhoeven]

Hi Joe,

On Tue, Oct 8, 2019 at 8:25 PM Joe Perches <joe@perches.com> wrote:
> On Tue, 2019-10-08 at 20:10 +0200, Geert Uytterhoeven wrote:
> > On Tue, Oct 8, 2019 at 7:02 PM Joe Perches <joe@perches.com> wrote:
> > > On Tue, 2019-10-08 at 17:28 +0200, Geert Uytterhoeven wrote:
> > > > On Tue, Oct 8, 2019 at 5:20 PM Joe Perches <joe@perches.com> wrote:
> > > > > On Tue, 2019-10-08 at 11:40 +0200, Geert Uytterhoeven wrote:
> > > > > > When reading a patch file from standard input, checkpatch calls it "Your
> > > > > > patch", and reports its state as:
> > > > > >
> > > > > >     Your patch has style problems, please review.
> > > > > >
> > > > > > or:
> > > > > >
> > > > > >     Your patch has no obvious style problems and is ready for submission.
> > > > > >
> > > > > > Hence when checking multiple patches by piping them to checkpatch, e.g.
> > > > > > when checking patchwork bundles using:
> > > > > >
> > > > > >     formail -s scripts/checkpatch.pl < bundle-foo.mbox
> > > > > >
> > > > > > it is difficult to identify which patches need to be reviewed and
> > > > > > improved.
> > > > > >
> > > > > > Fix this by replacing "Your patch" by the patch subject, if present.
> > > > > []
> > > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > > > []
> > > > > > @@ -1047,6 +1047,10 @@ for my $filename (@ARGV) {
> > > > > >       }
> > > > > >       while (<$FILE>) {
> > > > > >               chomp;
> > > > > > +             if ($vname eq 'Your patch') {
> > > > > > +                     my ($subject) = $_ =~ /^Subject:\s*(.*)/;
> > > > > > +                     $vname = '"' . $subject . '"' if $subject;
> > > > >
> > > > > Hi again Geert.
> > > > >
> > > > > Just some stylistic nits:
> > > > >
> > > > > $filename is not quoted so I think adding quotes
> > > > > before and after $subject may not be useful.
> > > >
> > > > Filename is indeed not quoted, but $git_commits{$filename} is.
> > >
> > > If I understand your use case, this will only show the last
> > > patch $subject of a bundle?
> >
> > False.
>
> Not really false, it's true.  Your use case just
> doesn't submit bundled patches as a single input
> to checkpatch.
>
> > "formail -s scripts/checkpatch.pl < bundle-foo.mbox" splits
> > "bundle-foo.mbox" in separate patches, and invokes
> > "scripts/checkpatch.pl" for each of them.
>
> Never used formail, it seems it was last updated in 2001.
>
> > > Also, it'll show things like "duplicate signature" when multiple
> > > patches are tested in a single bundle.
> >
> > False, due to the splitting by formail.
>
> Again, not false, just not your use.
>
> > > For instance, if I have a git format-patch series in an output
> > > directory and do
> > >
> > > $ cat <output_dir>/*.patch | ./scripts/checkpatch.pl
> > >
> > > Bad output happen.
> >
> > Yeah, because you're concatenating all patches.
> > Currently it works for single patches only.
> >
> > > Maybe this might be better:
> > > --- a/scripts/checkpatch.pl
> > > +++ b/scripts/checkpatch.pl
> > > @@ -2444,6 +2444,15 @@ sub process {
> > >
> > >                 my $rawline = $rawlines[$linenr - 1];
> > >
> > > +# if input from stdin, report the subject lines if they exist
> > > +               if ($filename eq '-' && !$quiet &&
> > > +                   $rawline =~ /^Subject:\s*(.*)/) {
> > > +                       report("stdin", "STDIN", '-' x length($1));
> > > +                       report("stdin", "STDIN", $1);
> > > +                       report("stdin", "STDIN", '-' x length($1));
> > > +                       %signatures = ();       # avoid duplicate signatures
> > > +               }
> > > +
> > >  # check if it's a mode change, rename or start of a patch
> > >                 if (!$in_commit_log &&
> > >                     ($line =~ /^ mode change [0-7]+ => [0-7]+ \S+\s*$/ ||
> >
> > Perhaps.  Just passing the patchwork bundle to checkpatch, and fixing
> > checkpatch to handle multiple patches in a single file was my first idea.
> > But it looked fragile, with too much state that needs to be reset.
> > I.e. the state is not limited to %signatures.  You also have to reset
> > $author inside process(), and probably a dozen other variables.
> > And make sure that future changes don't forget resetting all newly
> > introduced variables.
> >
> > Hence I settled for the solution using formail.
>
> I still think the patch I suggested is better as it
> functions for other use cases too.

I agree it would be better if checkpatch would handle the splitting in
patches itself, as that would be easier for the user.

However:
  1) That requires getting the state reset right,
  2) Using formail is the classical old UNIX way (combine small tools
     to get the job done ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] checkpatch: use patch subject when reading from stdin

[Joe Perches]

On Tue, 2019-10-08 at 20:39 +0200, Geert Uytterhoeven wrote:
[]
> > I still think the patch I suggested is better as it
> > functions for other use cases too.
> 
> I agree it would be better if checkpatch would handle the splitting in
> patches itself, as that would be easier for the user.
> 
> However:
>   1) That requires getting the state reset right,

Not really difficult and maybe not necessary.
The process subroutine contains very limited state.
The biggest issue is resetting the "$in_header_lines"
states and such when a new multi-patch bundle
start is detected.  I'm not sure it actually impacts
the checkpatch output much.  Maybe it'd not emit a
>75 character warning when scanning commit log messages.

>   2) Using formail is the classical old UNIX way (combine small tools
>      to get the job done ;-)

Which doesn't impact your use case as formail is
already running checkpatch multiple times.

cheers, Joe

Re: [PATCH] checkpatch: use patch subject when reading from stdin

[Geert Uytterhoeven]

Hi Joe,

On Tue, Oct 8, 2019 at 8:10 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Tue, Oct 8, 2019 at 7:02 PM Joe Perches <joe@perches.com> wrote:
> > On Tue, 2019-10-08 at 17:28 +0200, Geert Uytterhoeven wrote:
> > > On Tue, Oct 8, 2019 at 5:20 PM Joe Perches <joe@perches.com> wrote:
> > > > On Tue, 2019-10-08 at 11:40 +0200, Geert Uytterhoeven wrote:
> > > > > When reading a patch file from standard input, checkpatch calls it "Your
> > > > > patch", and reports its state as:
> > > > >
> > > > >     Your patch has style problems, please review.
> > > > >
> > > > > or:
> > > > >
> > > > >     Your patch has no obvious style problems and is ready for submission.
> > > > >
> > > > > Hence when checking multiple patches by piping them to checkpatch, e.g.
> > > > > when checking patchwork bundles using:
> > > > >
> > > > >     formail -s scripts/checkpatch.pl < bundle-foo.mbox
> > > > >
> > > > > it is difficult to identify which patches need to be reviewed and
> > > > > improved.
> > > > >
> > > > > Fix this by replacing "Your patch" by the patch subject, if present.
> > > > []
> > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > > []
> > > > > @@ -1047,6 +1047,10 @@ for my $filename (@ARGV) {
> > > > >       }
> > > > >       while (<$FILE>) {
> > > > >               chomp;
> > > > > +             if ($vname eq 'Your patch') {
> > > > > +                     my ($subject) = $_ =~ /^Subject:\s*(.*)/;
> > > > > +                     $vname = '"' . $subject . '"' if $subject;
> > > >
> > > > Hi again Geert.
> > > >
> > > > Just some stylistic nits:
> > > >
> > > > $filename is not quoted so I think adding quotes
> > > > before and after $subject may not be useful.
> > >
> > > Filename is indeed not quoted, but $git_commits{$filename} is.
> >
> > If I understand your use case, this will only show the last
> > patch $subject of a bundle?
>
> False.
> "formail -s scripts/checkpatch.pl < bundle-foo.mbox" splits
> "bundle-foo.mbox" in separate patches, and invokes
> "scripts/checkpatch.pl" for each of them.
>
> > Also, it'll show things like "duplicate signature" when multiple
> > patches are tested in a single bundle.
>
> False, due to the splitting by formail.
>
> > For instance, if I have a git format-patch series in an output
> > directory and do
> >
> > $ cat <output_dir>/*.patch | ./scripts/checkpatch.pl
> >
> > Bad output happen.
>
> Yeah, because you're concatenating all patches.
> Currently it works for single patches only.
>
> > Maybe this might be better:
>
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -2444,6 +2444,15 @@ sub process {
> >
> >                 my $rawline = $rawlines[$linenr - 1];
> >
> > +# if input from stdin, report the subject lines if they exist
> > +               if ($filename eq '-' && !$quiet &&
> > +                   $rawline =~ /^Subject:\s*(.*)/) {
> > +                       report("stdin", "STDIN", '-' x length($1));
> > +                       report("stdin", "STDIN", $1);
> > +                       report("stdin", "STDIN", '-' x length($1));
> > +                       %signatures = ();       # avoid duplicate signatures
> > +               }
> > +
> >  # check if it's a mode change, rename or start of a patch
> >                 if (!$in_commit_log &&
> >                     ($line =~ /^ mode change [0-7]+ => [0-7]+ \S+\s*$/ ||
>
> Perhaps.  Just passing the patchwork bundle to checkpatch, and fixing
> checkpatch to handle multiple patches in a single file was my first idea.
> But it looked fragile, with too much state that needs to be reset.
> I.e. the state is not limited to %signatures.  You also have to reset
> $author inside process(), and probably a dozen other variables.
> And make sure that future changes don't forget resetting all newly
> introduced variables.
>
> Hence I settled for the solution using formail.

I gave your solution a try.
It only enables the reset-on-next-patch feature when using stdin.
Thanks to the printed subject, it's now obvious to which patch a
message applies to.
However, the output is significantly different than when passing
a split patch series.  Can this be improved upon?

Note that the only reason I'm using stdin is that I use formail to split
a bundle in individual patches.  Once checkpatch supports bundles (or
mboxes) containing multiple patches, there's no longer a need for
using formail, and the reset-on-next-patch feature should be
enabled unconditionally.

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] checkpatch: use patch subject when reading from stdin

[Joe Perches]

On Mon, 2019-10-14 at 11:20 +0200, Geert Uytterhoeven wrote:
> Hi Joe,
> 
> On Tue, Oct 8, 2019 at 8:10 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, Oct 8, 2019 at 7:02 PM Joe Perches <joe@perches.com> wrote:
> > > On Tue, 2019-10-08 at 17:28 +0200, Geert Uytterhoeven wrote:
> > > > On Tue, Oct 8, 2019 at 5:20 PM Joe Perches <joe@perches.com> wrote:
> > > > > On Tue, 2019-10-08 at 11:40 +0200, Geert Uytterhoeven wrote:
> > > > > > When reading a patch file from standard input, checkpatch calls it "Your
> > > > > > patch", and reports its state as:
> > > > > > 
> > > > > >     Your patch has style problems, please review.
> > > > > > 
> > > > > > or:
> > > > > > 
> > > > > >     Your patch has no obvious style problems and is ready for submission.
> > > > > > 
> > > > > > Hence when checking multiple patches by piping them to checkpatch, e.g.
> > > > > > when checking patchwork bundles using:
> > > > > > 
> > > > > >     formail -s scripts/checkpatch.pl < bundle-foo.mbox
> > > > > > 
> > > > > > it is difficult to identify which patches need to be reviewed and
> > > > > > improved.
> > > > > > 
> > > > > > Fix this by replacing "Your patch" by the patch subject, if present.
> > > > > []
> > > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > > > []
> > > > > > @@ -1047,6 +1047,10 @@ for my $filename (@ARGV) {
> > > > > >       }
> > > > > >       while (<$FILE>) {
> > > > > >               chomp;
> > > > > > +             if ($vname eq 'Your patch') {
> > > > > > +                     my ($subject) = $_ =~ /^Subject:\s*(.*)/;
> > > > > > +                     $vname = '"' . $subject . '"' if $subject;
> > > > > 
> > > > > Hi again Geert.
> > > > > 
> > > > > Just some stylistic nits:
> > > > > 
> > > > > $filename is not quoted so I think adding quotes
> > > > > before and after $subject may not be useful.
> > > > 
> > > > Filename is indeed not quoted, but $git_commits{$filename} is.
> > > 
> > > If I understand your use case, this will only show the last
> > > patch $subject of a bundle?
> > 
> > False.
> > "formail -s scripts/checkpatch.pl < bundle-foo.mbox" splits
> > "bundle-foo.mbox" in separate patches, and invokes
> > "scripts/checkpatch.pl" for each of them.
> > 
> > > Also, it'll show things like "duplicate signature" when multiple
> > > patches are tested in a single bundle.
> > 
> > False, due to the splitting by formail.
> > 
> > > For instance, if I have a git format-patch series in an output
> > > directory and do
> > > 
> > > $ cat <output_dir>/*.patch | ./scripts/checkpatch.pl
> > > 
> > > Bad output happen.
> > 
> > Yeah, because you're concatenating all patches.
> > Currently it works for single patches only.
> > 
> > > Maybe this might be better:
> > > --- a/scripts/checkpatch.pl
> > > +++ b/scripts/checkpatch.pl
> > > @@ -2444,6 +2444,15 @@ sub process {
> > > 
> > >                 my $rawline = $rawlines[$linenr - 1];
> > > 
> > > +# if input from stdin, report the subject lines if they exist
> > > +               if ($filename eq '-' && !$quiet &&
> > > +                   $rawline =~ /^Subject:\s*(.*)/) {
> > > +                       report("stdin", "STDIN", '-' x length($1));
> > > +                       report("stdin", "STDIN", $1);
> > > +                       report("stdin", "STDIN", '-' x length($1));
> > > +                       %signatures = ();       # avoid duplicate signatures
> > > +               }
> > > +
> > >  # check if it's a mode change, rename or start of a patch
> > >                 if (!$in_commit_log &&
> > >                     ($line =~ /^ mode change [0-7]+ => [0-7]+ \S+\s*$/ ||
> > 
> > Perhaps.  Just passing the patchwork bundle to checkpatch, and fixing
> > checkpatch to handle multiple patches in a single file was my first idea.
> > But it looked fragile, with too much state that needs to be reset.
> > I.e. the state is not limited to %signatures.  You also have to reset
> > $author inside process(), and probably a dozen other variables.
> > And make sure that future changes don't forget resetting all newly
> > introduced variables.
> > 
> > Hence I settled for the solution using formail.
> 
> I gave your solution a try.
> It only enables the reset-on-next-patch feature when using stdin.
> Thanks to the printed subject, it's now obvious to which patch a
> message applies to.
> However, the output is significantly different than when passing
> a split patch series.  Can this be improved upon?
> 
> Note that the only reason I'm using stdin is that I use formail to split
> a bundle in individual patches.  Once checkpatch supports bundles (or
> mboxes) containing multiple patches, there's no longer a need for
> using formail, and the reset-on-next-patch feature should be
> enabled unconditionally.

Using your collection of little tools idea,
why not write a trivial script like:

	grep "^Subject:" $1
	checkpatch.pl $1

and use that as the command line for formail
instead of adding unnecessary complexity to
checkpatch?

Re: [PATCH] checkpatch: use patch subject when reading from stdin

[Geert Uytterhoeven]

Hi Joe,

On Mon, Oct 14, 2019 at 4:48 PM Joe Perches <joe@perches.com> wrote:
> On Mon, 2019-10-14 at 11:20 +0200, Geert Uytterhoeven wrote:
> > On Tue, Oct 8, 2019 at 8:10 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Tue, Oct 8, 2019 at 7:02 PM Joe Perches <joe@perches.com> wrote:
> > > > On Tue, 2019-10-08 at 17:28 +0200, Geert Uytterhoeven wrote:
> > > > > On Tue, Oct 8, 2019 at 5:20 PM Joe Perches <joe@perches.com> wrote:
> > > > > > On Tue, 2019-10-08 at 11:40 +0200, Geert Uytterhoeven wrote:
> > > > > > > When reading a patch file from standard input, checkpatch calls it "Your
> > > > > > > patch", and reports its state as:
> > > > > > >
> > > > > > >     Your patch has style problems, please review.
> > > > > > >
> > > > > > > or:
> > > > > > >
> > > > > > >     Your patch has no obvious style problems and is ready for submission.
> > > > > > >
> > > > > > > Hence when checking multiple patches by piping them to checkpatch, e.g.
> > > > > > > when checking patchwork bundles using:
> > > > > > >
> > > > > > >     formail -s scripts/checkpatch.pl < bundle-foo.mbox
> > > > > > >
> > > > > > > it is difficult to identify which patches need to be reviewed and
> > > > > > > improved.
> > > > > > >
> > > > > > > Fix this by replacing "Your patch" by the patch subject, if present.
> > > > > > []
> > > > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > > > > []
> > > > > > > @@ -1047,6 +1047,10 @@ for my $filename (@ARGV) {
> > > > > > >       }
> > > > > > >       while (<$FILE>) {
> > > > > > >               chomp;
> > > > > > > +             if ($vname eq 'Your patch') {
> > > > > > > +                     my ($subject) = $_ =~ /^Subject:\s*(.*)/;
> > > > > > > +                     $vname = '"' . $subject . '"' if $subject;
> > > > > >
> > > > > > Hi again Geert.
> > > > > >
> > > > > > Just some stylistic nits:
> > > > > >
> > > > > > $filename is not quoted so I think adding quotes
> > > > > > before and after $subject may not be useful.
> > > > >
> > > > > Filename is indeed not quoted, but $git_commits{$filename} is.
> > > >
> > > > If I understand your use case, this will only show the last
> > > > patch $subject of a bundle?
> > >
> > > False.
> > > "formail -s scripts/checkpatch.pl < bundle-foo.mbox" splits
> > > "bundle-foo.mbox" in separate patches, and invokes
> > > "scripts/checkpatch.pl" for each of them.
> > >
> > > > Also, it'll show things like "duplicate signature" when multiple
> > > > patches are tested in a single bundle.
> > >
> > > False, due to the splitting by formail.
> > >
> > > > For instance, if I have a git format-patch series in an output
> > > > directory and do
> > > >
> > > > $ cat <output_dir>/*.patch | ./scripts/checkpatch.pl
> > > >
> > > > Bad output happen.
> > >
> > > Yeah, because you're concatenating all patches.
> > > Currently it works for single patches only.
> > >
> > > > Maybe this might be better:
> > > > --- a/scripts/checkpatch.pl
> > > > +++ b/scripts/checkpatch.pl
> > > > @@ -2444,6 +2444,15 @@ sub process {
> > > >
> > > >                 my $rawline = $rawlines[$linenr - 1];
> > > >
> > > > +# if input from stdin, report the subject lines if they exist
> > > > +               if ($filename eq '-' && !$quiet &&
> > > > +                   $rawline =~ /^Subject:\s*(.*)/) {
> > > > +                       report("stdin", "STDIN", '-' x length($1));
> > > > +                       report("stdin", "STDIN", $1);
> > > > +                       report("stdin", "STDIN", '-' x length($1));
> > > > +                       %signatures = ();       # avoid duplicate signatures
> > > > +               }
> > > > +
> > > >  # check if it's a mode change, rename or start of a patch
> > > >                 if (!$in_commit_log &&
> > > >                     ($line =~ /^ mode change [0-7]+ => [0-7]+ \S+\s*$/ ||
> > >
> > > Perhaps.  Just passing the patchwork bundle to checkpatch, and fixing
> > > checkpatch to handle multiple patches in a single file was my first idea.
> > > But it looked fragile, with too much state that needs to be reset.
> > > I.e. the state is not limited to %signatures.  You also have to reset
> > > $author inside process(), and probably a dozen other variables.
> > > And make sure that future changes don't forget resetting all newly
> > > introduced variables.
> > >
> > > Hence I settled for the solution using formail.
> >
> > I gave your solution a try.
> > It only enables the reset-on-next-patch feature when using stdin.
> > Thanks to the printed subject, it's now obvious to which patch a
> > message applies to.
> > However, the output is significantly different than when passing
> > a split patch series.  Can this be improved upon?
> >
> > Note that the only reason I'm using stdin is that I use formail to split
> > a bundle in individual patches.  Once checkpatch supports bundles (or
> > mboxes) containing multiple patches, there's no longer a need for
> > using formail, and the reset-on-next-patch feature should be
> > enabled unconditionally.
>
> Using your collection of little tools idea,
> why not write a trivial script like:
>
>         grep "^Subject:" $1
>         checkpatch.pl $1
>
> and use that as the command line for formail
> instead of adding unnecessary complexity to
> checkpatch?

That would be another possibility.

But given more maintainers are starting to apply patchwork bundles (cfr.
the workflows discussions), it makes sense to make their lives easier.

This is also useful for maintainers who save all patches to apply into a
single mbox, and run checkpatch+git-am on that.

Summarized: git-am handles multiple patches, checkpatch requires
splitting.

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] checkpatch: use patch subject when reading from stdin

[Joe Perches]

On Tue, 2019-10-15 at 08:49 +0200, Geert Uytterhoeven wrote:
> Hi Joe,

Rehi Geert.

> On Mon, Oct 14, 2019 at 4:48 PM Joe Perches <joe@perches.com> wrote:
> > On Mon, 2019-10-14 at 11:20 +0200, Geert Uytterhoeven wrote:
[]
> > > I gave your solution a try.
> > > It only enables the reset-on-next-patch feature when using stdin.
> > > Thanks to the printed subject, it's now obvious to which patch a
> > > message applies to.
> > > However, the output is significantly different than when passing
> > > a split patch series.  Can this be improved upon?
> > > 
> > > Note that the only reason I'm using stdin is that I use formail to split
> > > a bundle in individual patches.  Once checkpatch supports bundles (or
> > > mboxes) containing multiple patches, there's no longer a need for
> > > using formail, and the reset-on-next-patch feature should be
> > > enabled unconditionally.
> > 
> > Using your collection of little tools idea,
> > why not write a trivial script like:
> > 
> >         grep "^Subject:" $1
> >         checkpatch.pl $1
> > 
> > and use that as the command line for formail
> > instead of adding unnecessary complexity to
> > checkpatch?
> 
> That would be another possibility.
> 
> But given more maintainers are starting to apply patchwork bundles (cfr.
> the workflows discussions), it makes sense to make their lives easier.

But given this particular change only works for stdin, then this
patch splitting idea wouldn't generically work.

> This is also useful for maintainers who save all patches to apply into a
> single mbox, and run checkpatch+git-am on that.

Which also wouldn't generally work for checkpatch <mbox>

> Summarized: git-am handles multiple patches, checkpatch requires
> splitting.

I still think it's better to introduce YA script that disaggregates
aggregated patches/emails and feeds each individual patch to
checkpatch rather than making checkpatch learn how to disaggregate.

Using "git mailsplit" as part of some additional script could work.

Re: [PATCH] checkpatch: use patch subject when reading from stdin

[Geert Uytterhoeven]

Hi Joe,

On Tue, Oct 15, 2019 at 5:50 PM Joe Perches <joe@perches.com> wrote:
> On Tue, 2019-10-15 at 08:49 +0200, Geert Uytterhoeven wrote:
> > On Mon, Oct 14, 2019 at 4:48 PM Joe Perches <joe@perches.com> wrote:
> > > On Mon, 2019-10-14 at 11:20 +0200, Geert Uytterhoeven wrote:
> []
> > > > I gave your solution a try.
> > > > It only enables the reset-on-next-patch feature when using stdin.
> > > > Thanks to the printed subject, it's now obvious to which patch a
> > > > message applies to.
> > > > However, the output is significantly different than when passing
> > > > a split patch series.  Can this be improved upon?
> > > >
> > > > Note that the only reason I'm using stdin is that I use formail to split
> > > > a bundle in individual patches.  Once checkpatch supports bundles (or
> > > > mboxes) containing multiple patches, there's no longer a need for
> > > > using formail, and the reset-on-next-patch feature should be
> > > > enabled unconditionally.
> > >
> > > Using your collection of little tools idea,
> > > why not write a trivial script like:
> > >
> > >         grep "^Subject:" $1
> > >         checkpatch.pl $1
> > >
> > > and use that as the command line for formail
> > > instead of adding unnecessary complexity to
> > > checkpatch?
> >
> > That would be another possibility.
> >
> > But given more maintainers are starting to apply patchwork bundles (cfr.
> > the workflows discussions), it makes sense to make their lives easier.
>
> But given this particular change only works for stdin, then this
> patch splitting idea wouldn't generically work.
>
> > This is also useful for maintainers who save all patches to apply into a
> > single mbox, and run checkpatch+git-am on that.
>
> Which also wouldn't generally work for checkpatch <mbox>

formail -s scripts/checkpatch.pl < <mbox>

> > Summarized: git-am handles multiple patches, checkpatch requires
> > splitting.
>
> I still think it's better to introduce YA script that disaggregates
> aggregated patches/emails and feeds each individual patch to
> checkpatch rather than making checkpatch learn how to disaggregate.
>
> Using "git mailsplit" as part of some additional script could work.

Thanks, didn't know git was assimilating formail functionality...

[reading git-mailsplit(1)]

The advantage of formail over git-mailsplit is that the former doesn't
create a bunch of files that need to be stored in a temporary place,
and cleant up afterwards.
But hey, that can be handled in yet-another-script...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] checkpatch: use patch subject when reading from stdin

[Joe Perches]

On Tue, 2019-10-15 at 18:40 +0200, Geert Uytterhoeven wrote:
> The advantage of formail over git-mailsplit is that the former doesn't
> create a bunch of files that need to be stored in a temporary place,
> and cleant up afterwards.
> But hey, that can be handled in yet-another-script...

Or yet-another-git-hook.

Re: [PATCH] checkpatch: use patch subject when reading from stdin

[Geert Uytterhoeven]

Hi Joe,

On Tue, Oct 15, 2019 at 7:24 PM Joe Perches <joe@perches.com> wrote:
> On Tue, 2019-10-15 at 18:40 +0200, Geert Uytterhoeven wrote:
> > The advantage of formail over git-mailsplit is that the former doesn't
> > create a bunch of files that need to be stored in a temporary place,
> > and cleant up afterwards.
> > But hey, that can be handled in yet-another-script...
>
> Or yet-another-git-hook.

Too late: thou maintainer shall run checkpatch before  applying patches.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds