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

[Manos Pitsidianakis <manos.pitsidianakis@linaro.org>]

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