1. Patchew
  2. QEMU
  3. View series

[PATCH v1] scripts/checkpatch.pl: check for placeholders in cover letter patches

Manos Pitsidianakis posted 1 patch 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240130075615.181040-1-manos.pitsidianakis@linaro.org
There is a newer version of this series
scripts/checkpatch.pl | 14 ++++++++++++++
1 file changed, 14 insertions(+)
[PATCH v1] scripts/checkpatch.pl: check for placeholders in cover letter patches
Posted by Manos Pitsidianakis 10 months ago 
Check if a file argument is a cover letter patch produced by
git-format-patch --cover-letter; It is initialized with subject suffix "
*** SUBJECT HERE ***" and body prefix " *** BLURB HERE ***". If they
exist, warn the user.

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
 scripts/checkpatch.pl | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7026895074..34f12c9848 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1650,6 +1650,20 @@ sub process {
 			$non_utf8_charset = 1;
 		}
 
+# Check if this is a cover letter patch produced by git-format-patch
+# --cover-letter; It is initialized with subject suffix
+# " *** SUBJECT HERE ***" and body prefix " *** BLURB HERE ***"
+		if ($in_header_lines &&
+		    $rawline =~ /^Subject:.+[*]{3} SUBJECT HERE [*]{3}\s*$/) {
+        WARN("Patch appears to be a cover letter with uninitialized subject" .
+             " '*** SUBJECT HERE ***'\n$hereline\n");
+		}
+
+		if ($rawline =~ /^[*]{3} BLURB HERE [*]{3}\s*$/) {
+        WARN("Patch appears to be a cover letter with leftover placeholder " .
+             "text '*** BLURB HERE ***'\n$hereline\n");
+		}
+
 		if ($in_commit_log && $non_utf8_charset && $realfile =~ /^$/ &&
 		    $rawline =~ /$NON_ASCII_UTF8/) {
 			WARN("8-bit UTF-8 used in possible commit log\n" . $herecurr);

base-commit: 11be70677c70fdccd452a3233653949b79e97908
-- 
γαῖα πυρί μιχθήτω
Re: [PATCH v1] scripts/checkpatch.pl: check for placeholders in cover letter patches
Posted by Daniel P. Berrangé 10 months ago 
On Tue, Jan 30, 2024 at 09:56:15AM +0200, Manos Pitsidianakis wrote:
> Check if a file argument is a cover letter patch produced by
> git-format-patch --cover-letter; It is initialized with subject suffix "
> *** SUBJECT HERE ***" and body prefix " *** BLURB HERE ***". If they
> exist, warn the user.
> 
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
>  scripts/checkpatch.pl | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7026895074..34f12c9848 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1650,6 +1650,20 @@ sub process {
>  			$non_utf8_charset = 1;
>  		}
>  
> +# Check if this is a cover letter patch produced by git-format-patch
> +# --cover-letter; It is initialized with subject suffix
> +# " *** SUBJECT HERE ***" and body prefix " *** BLURB HERE ***"
> +		if ($in_header_lines &&
> +		    $rawline =~ /^Subject:.+[*]{3} SUBJECT HERE [*]{3}\s*$/) {
> +        WARN("Patch appears to be a cover letter with uninitialized subject" .
> +             " '*** SUBJECT HERE ***'\n$hereline\n");
> +		}
> +
> +		if ($rawline =~ /^[*]{3} BLURB HERE [*]{3}\s*$/) {
> +        WARN("Patch appears to be a cover letter with leftover placeholder " .
> +             "text '*** BLURB HERE ***'\n$hereline\n");
> +		}

Indentation here is totally off

> +
>  		if ($in_commit_log && $non_utf8_charset && $realfile =~ /^$/ &&
>  		    $rawline =~ /$NON_ASCII_UTF8/) {
>  			WARN("8-bit UTF-8 used in possible commit log\n" . $herecurr);
> 
> base-commit: 11be70677c70fdccd452a3233653949b79e97908
> -- 
> γαῖα πυρί μιχθήτω
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH v1] scripts/checkpatch.pl: check for placeholders in cover letter patches
Posted by Manos Pitsidianakis 10 months ago 
On Tue, 30 Jan 2024 at 10:12, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Tue, Jan 30, 2024 at 09:56:15AM +0200, Manos Pitsidianakis wrote:
> > Check if a file argument is a cover letter patch produced by
> > git-format-patch --cover-letter; It is initialized with subject suffix "
> > *** SUBJECT HERE ***" and body prefix " *** BLURB HERE ***". If they
> > exist, warn the user.
> >
> > Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > ---
> >  scripts/checkpatch.pl | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 7026895074..34f12c9848 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -1650,6 +1650,20 @@ sub process {
> >                       $non_utf8_charset = 1;
> >               }
> >
> > +# Check if this is a cover letter patch produced by git-format-patch
> > +# --cover-letter; It is initialized with subject suffix
> > +# " *** SUBJECT HERE ***" and body prefix " *** BLURB HERE ***"
> > +             if ($in_header_lines &&
> > +                 $rawline =~ /^Subject:.+[*]{3} SUBJECT HERE [*]{3}\s*$/) {
> > +        WARN("Patch appears to be a cover letter with uninitialized subject" .
> > +             " '*** SUBJECT HERE ***'\n$hereline\n");
> > +             }
> > +
> > +             if ($rawline =~ /^[*]{3} BLURB HERE [*]{3}\s*$/) {
> > +        WARN("Patch appears to be a cover letter with leftover placeholder " .
> > +             "text '*** BLURB HERE ***'\n$hereline\n");
> > +             }
>
> Indentation here is totally off

It only seems that way because the pre-existing lines use tabs, while
I used spaces, according to the QEMU Coding style:

> QEMU indents are four spaces. Tabs are never used, except in Makefiles where they have been irreversibly coded into the syntax.

> > +
> >               if ($in_commit_log && $non_utf8_charset && $realfile =~ /^$/ &&
> >                   $rawline =~ /$NON_ASCII_UTF8/) {
> >                       WARN("8-bit UTF-8 used in possible commit log\n" . $herecurr);
> >
> > base-commit: 11be70677c70fdccd452a3233653949b79e97908
> > --
> > γαῖα πυρί μιχθήτω
> >
>
> With regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>
Re: [PATCH v1] scripts/checkpatch.pl: check for placeholders in cover letter patches
Posted by Daniel P. Berrangé 10 months ago 
On Tue, Jan 30, 2024 at 10:51:58AM +0200, Manos Pitsidianakis wrote:
> On Tue, 30 Jan 2024 at 10:12, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Tue, Jan 30, 2024 at 09:56:15AM +0200, Manos Pitsidianakis wrote:
> > > Check if a file argument is a cover letter patch produced by
> > > git-format-patch --cover-letter; It is initialized with subject suffix "
> > > *** SUBJECT HERE ***" and body prefix " *** BLURB HERE ***". If they
> > > exist, warn the user.
> > >
> > > Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > > ---
> > >  scripts/checkpatch.pl | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > >
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > index 7026895074..34f12c9848 100755
> > > --- a/scripts/checkpatch.pl
> > > +++ b/scripts/checkpatch.pl
> > > @@ -1650,6 +1650,20 @@ sub process {
> > >                       $non_utf8_charset = 1;
> > >               }
> > >
> > > +# Check if this is a cover letter patch produced by git-format-patch
> > > +# --cover-letter; It is initialized with subject suffix
> > > +# " *** SUBJECT HERE ***" and body prefix " *** BLURB HERE ***"
> > > +             if ($in_header_lines &&
> > > +                 $rawline =~ /^Subject:.+[*]{3} SUBJECT HERE [*]{3}\s*$/) {
> > > +        WARN("Patch appears to be a cover letter with uninitialized subject" .
> > > +             " '*** SUBJECT HERE ***'\n$hereline\n");
> > > +             }
> > > +
> > > +             if ($rawline =~ /^[*]{3} BLURB HERE [*]{3}\s*$/) {
> > > +        WARN("Patch appears to be a cover letter with leftover placeholder " .
> > > +             "text '*** BLURB HERE ***'\n$hereline\n");
> > > +             }
> >
> > Indentation here is totally off
> 
> It only seems that way because the pre-existing lines use tabs, while
> I used spaces, according to the QEMU Coding style:

It is more important to be consistent within a single function.

Regardless of that though, the indent is still broken because the body
of the 'if' condition is indented /less/ than the 'if' condition itself.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH v1] scripts/checkpatch.pl: check for placeholders in cover letter patches
Posted by Manos Pitsidianakis 10 months ago 
On Tue, 30 Jan 2024 11:47, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
>On Tue, Jan 30, 2024 at 10:51:58AM +0200, Manos Pitsidianakis wrote:
>> On Tue, 30 Jan 2024 at 10:12, Daniel P. Berrangé <berrange@redhat.com> wrote:
>> >
>> > On Tue, Jan 30, 2024 at 09:56:15AM +0200, Manos Pitsidianakis wrote:
>> > > Check if a file argument is a cover letter patch produced by
>> > > git-format-patch --cover-letter; It is initialized with subject suffix "
>> > > *** SUBJECT HERE ***" and body prefix " *** BLURB HERE ***". If they
>> > > exist, warn the user.
>> > >
>> > > Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>> > > ---
>> > >  scripts/checkpatch.pl | 14 ++++++++++++++
>> > >  1 file changed, 14 insertions(+)
>> > >
>> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> > > index 7026895074..34f12c9848 100755
>> > > --- a/scripts/checkpatch.pl
>> > > +++ b/scripts/checkpatch.pl
>> > > @@ -1650,6 +1650,20 @@ sub process {
>> > >                       $non_utf8_charset = 1;
>> > >               }
>> > >
>> > > +# Check if this is a cover letter patch produced by git-format-patch
>> > > +# --cover-letter; It is initialized with subject suffix
>> > > +# " *** SUBJECT HERE ***" and body prefix " *** BLURB HERE ***"
>> > > +             if ($in_header_lines &&
>> > > +                 $rawline =~ /^Subject:.+[*]{3} SUBJECT HERE [*]{3}\s*$/) {
>> > > +        WARN("Patch appears to be a cover letter with uninitialized subject" .
>> > > +             " '*** SUBJECT HERE ***'\n$hereline\n");
>> > > +             }
>> > > +
>> > > +             if ($rawline =~ /^[*]{3} BLURB HERE [*]{3}\s*$/) {
>> > > +        WARN("Patch appears to be a cover letter with leftover placeholder " .
>> > > +             "text '*** BLURB HERE ***'\n$hereline\n");
>> > > +             }
>> >
>> > Indentation here is totally off
>> 
>> It only seems that way because the pre-existing lines use tabs, while
>> I used spaces, according to the QEMU Coding style:
>
>It is more important to be consistent within a single function.
>
>Regardless of that though, the indent is still broken because the body
>of the 'if' condition is indented /less/ than the 'if' condition itself.

Well not really, that's because my editor replaced the tabs when quoting 
your e-mail.

Anyway, I will respin a v2 with just tabs for indentation right away.


Thanks!




>With regards,
>Daniel
>-- 
>|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>
Re: [PATCH v1] scripts/checkpatch.pl: check for placeholders in cover letter patches
Posted by Daniel P. Berrangé 10 months ago 
On Tue, Jan 30, 2024 at 11:54:51AM +0200, Manos Pitsidianakis wrote:
> On Tue, 30 Jan 2024 11:47, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
> > On Tue, Jan 30, 2024 at 10:51:58AM +0200, Manos Pitsidianakis wrote:
> > > On Tue, 30 Jan 2024 at 10:12, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > >
> > > > On Tue, Jan 30, 2024 at 09:56:15AM +0200, Manos Pitsidianakis wrote:
> > > > > Check if a file argument is a cover letter patch produced by
> > > > > git-format-patch --cover-letter; It is initialized with subject suffix "
> > > > > *** SUBJECT HERE ***" and body prefix " *** BLURB HERE ***". If they
> > > > > exist, warn the user.
> > > > >
> > > > > Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> > > > > ---
> > > > >  scripts/checkpatch.pl | 14 ++++++++++++++
> > > > >  1 file changed, 14 insertions(+)
> > > > >
> > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > > > index 7026895074..34f12c9848 100755
> > > > > --- a/scripts/checkpatch.pl
> > > > > +++ b/scripts/checkpatch.pl
> > > > > @@ -1650,6 +1650,20 @@ sub process {
> > > > >                       $non_utf8_charset = 1;
> > > > >               }
> > > > >
> > > > > +# Check if this is a cover letter patch produced by git-format-patch
> > > > > +# --cover-letter; It is initialized with subject suffix
> > > > > +# " *** SUBJECT HERE ***" and body prefix " *** BLURB HERE ***"
> > > > > +             if ($in_header_lines &&
> > > > > +                 $rawline =~ /^Subject:.+[*]{3} SUBJECT HERE [*]{3}\s*$/) {
> > > > > +        WARN("Patch appears to be a cover letter with uninitialized subject" .
> > > > > +             " '*** SUBJECT HERE ***'\n$hereline\n");
> > > > > +             }
> > > > > +
> > > > > +             if ($rawline =~ /^[*]{3} BLURB HERE [*]{3}\s*$/) {
> > > > > +        WARN("Patch appears to be a cover letter with leftover placeholder " .
> > > > > +             "text '*** BLURB HERE ***'\n$hereline\n");
> > > > > +             }
> > > >
> > > > Indentation here is totally off
> > > 
> > > It only seems that way because the pre-existing lines use tabs, while
> > > I used spaces, according to the QEMU Coding style:
> > 
> > It is more important to be consistent within a single function.
> > 
> > Regardless of that though, the indent is still broken because the body
> > of the 'if' condition is indented /less/ than the 'if' condition itself.
> 
> Well not really, that's because my editor replaced the tabs when quoting
> your e-mail.

The under-indentation of the WARN statement has nothing to do with
reply quoting, it is visible in the initial patch you submitted:

  https://lists.nongnu.org/archive/html/qemu-devel/2024-01/msg06216.html


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH v1] scripts/checkpatch.pl: check for placeholders in cover letter patches
Posted by Manos Pitsidianakis 10 months ago 
On Tue, 30 Jan 2024 11:59, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
>On Tue, Jan 30, 2024 at 11:54:51AM +0200, Manos Pitsidianakis wrote:
>> On Tue, 30 Jan 2024 11:47, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
>> > On Tue, Jan 30, 2024 at 10:51:58AM +0200, Manos Pitsidianakis wrote:
>> > > On Tue, 30 Jan 2024 at 10:12, Daniel P. Berrangé <berrange@redhat.com> wrote:
>> > > >
>> > > > On Tue, Jan 30, 2024 at 09:56:15AM +0200, Manos Pitsidianakis wrote:
>> > > > > Check if a file argument is a cover letter patch produced by
>> > > > > git-format-patch --cover-letter; It is initialized with subject suffix "
>> > > > > *** SUBJECT HERE ***" and body prefix " *** BLURB HERE ***". If they
>> > > > > exist, warn the user.
>> > > > >
>> > > > > Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>> > > > > ---
>> > > > >  scripts/checkpatch.pl | 14 ++++++++++++++
>> > > > >  1 file changed, 14 insertions(+)
>> > > > >
>> > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> > > > > index 7026895074..34f12c9848 100755
>> > > > > --- a/scripts/checkpatch.pl
>> > > > > +++ b/scripts/checkpatch.pl
>> > > > > @@ -1650,6 +1650,20 @@ sub process {
>> > > > >                       $non_utf8_charset = 1;
>> > > > >               }
>> > > > >
>> > > > > +# Check if this is a cover letter patch produced by git-format-patch
>> > > > > +# --cover-letter; It is initialized with subject suffix
>> > > > > +# " *** SUBJECT HERE ***" and body prefix " *** BLURB HERE ***"
>> > > > > +             if ($in_header_lines &&
>> > > > > +                 $rawline =~ /^Subject:.+[*]{3} SUBJECT HERE [*]{3}\s*$/) {
>> > > > > +        WARN("Patch appears to be a cover letter with uninitialized subject" .
>> > > > > +             " '*** SUBJECT HERE ***'\n$hereline\n");
>> > > > > +             }
>> > > > > +
>> > > > > +             if ($rawline =~ /^[*]{3} BLURB HERE [*]{3}\s*$/) {
>> > > > > +        WARN("Patch appears to be a cover letter with leftover placeholder " .
>> > > > > +             "text '*** BLURB HERE ***'\n$hereline\n");
>> > > > > +             }
>> > > >
>> > > > Indentation here is totally off
>> > > 
>> > > It only seems that way because the pre-existing lines use tabs, while
>> > > I used spaces, according to the QEMU Coding style:
>> > 
>> > It is more important to be consistent within a single function.
>> > 
>> > Regardless of that though, the indent is still broken because the body
>> > of the 'if' condition is indented /less/ than the 'if' condition itself.
>> 
>> Well not really, that's because my editor replaced the tabs when quoting
>> your e-mail.
>
>The under-indentation of the WARN statement has nothing to do with
>reply quoting, it is visible in the initial patch you submitted:
>
>  https://lists.nongnu.org/archive/html/qemu-devel/2024-01/msg06216.html

The html in that link has expanded tabs to 8 spaces :) And QEMU coding 
style says an indentation level is 4 spaces.

This is one of the reasons tabs should not be used for indentation at 
all. It shows up different in different contexts.

Anyway, it's not very important in any case. Besides, I have superseded 
this patch with a v2.

Thanks,
Manos

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.