| Message ID | 20240130075615.181040-1-manos.pitsidianakis@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] scripts/checkpatch.pl: check for placeholders in cover letter patches | expand |
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
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 :| >
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
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 :| >
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
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
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);
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(+) base-commit: 11be70677c70fdccd452a3233653949b79e97908