* [PATCH v1] scripts/checkpatch.pl: check for placeholders in cover letter patches @ 2024-01-30 7:56 Manos Pitsidianakis 2024-01-30 8:12 ` Daniel P. Berrangé 0 siblings, 1 reply; 7+ messages in thread From: Manos Pitsidianakis @ 2024-01-30 7:56 UTC (permalink / raw) To: qemu-devel Cc: Alex Bennée, Philippe Mathieu-Daudé, Peter Maydell, Daniel P. Berrangé, Paolo Bonzini, Stefan Hajnoczi 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 -- γαῖα πυρί μιχθήτω ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v1] scripts/checkpatch.pl: check for placeholders in cover letter patches 2024-01-30 7:56 [PATCH v1] scripts/checkpatch.pl: check for placeholders in cover letter patches Manos Pitsidianakis @ 2024-01-30 8:12 ` Daniel P. Berrangé 2024-01-30 8:51 ` Manos Pitsidianakis 0 siblings, 1 reply; 7+ messages in thread From: Daniel P. Berrangé @ 2024-01-30 8:12 UTC (permalink / raw) To: Manos Pitsidianakis Cc: qemu-devel, Alex Bennée, Philippe Mathieu-Daudé, Peter Maydell, Paolo Bonzini, Stefan Hajnoczi 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 :| ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1] scripts/checkpatch.pl: check for placeholders in cover letter patches 2024-01-30 8:12 ` Daniel P. Berrangé @ 2024-01-30 8:51 ` Manos Pitsidianakis 2024-01-30 9:47 ` Daniel P. Berrangé 0 siblings, 1 reply; 7+ messages in thread From: Manos Pitsidianakis @ 2024-01-30 8:51 UTC (permalink / raw) To: Daniel P. Berrangé Cc: qemu-devel, Alex Bennée, Philippe Mathieu-Daudé, Peter Maydell, Paolo Bonzini, Stefan Hajnoczi 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 :| > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1] scripts/checkpatch.pl: check for placeholders in cover letter patches 2024-01-30 8:51 ` Manos Pitsidianakis @ 2024-01-30 9:47 ` Daniel P. Berrangé 2024-01-30 9:54 ` Manos Pitsidianakis 0 siblings, 1 reply; 7+ messages in thread From: Daniel P. Berrangé @ 2024-01-30 9:47 UTC (permalink / raw) To: Manos Pitsidianakis Cc: qemu-devel, Alex Bennée, Philippe Mathieu-Daudé, Peter Maydell, Paolo Bonzini, Stefan Hajnoczi 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 :| ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1] scripts/checkpatch.pl: check for placeholders in cover letter patches 2024-01-30 9:47 ` Daniel P. Berrangé @ 2024-01-30 9:54 ` Manos Pitsidianakis 2024-01-30 9:59 ` Daniel P. Berrangé 0 siblings, 1 reply; 7+ messages in thread From: Manos Pitsidianakis @ 2024-01-30 9:54 UTC (permalink / raw) To: Daniel P. Berrangé Cc: qemu-devel, Alex Benné e, Philippe Mathieu-Daudé , Peter Maydell, Paolo Bonzini, Stefan Hajnoczi 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 :| > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1] scripts/checkpatch.pl: check for placeholders in cover letter patches 2024-01-30 9:54 ` Manos Pitsidianakis @ 2024-01-30 9:59 ` Daniel P. Berrangé 2024-01-30 10:17 ` Manos Pitsidianakis 0 siblings, 1 reply; 7+ messages in thread From: Daniel P. Berrangé @ 2024-01-30 9:59 UTC (permalink / raw) To: Manos Pitsidianakis Cc: qemu-devel, Alex Benné e, Philippe Mathieu-Daudé, Peter Maydell, Paolo Bonzini, Stefan Hajnoczi 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 :| ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1] scripts/checkpatch.pl: check for placeholders in cover letter patches 2024-01-30 9:59 ` Daniel P. Berrangé @ 2024-01-30 10:17 ` Manos Pitsidianakis 0 siblings, 0 replies; 7+ messages in thread From: Manos Pitsidianakis @ 2024-01-30 10:17 UTC (permalink / raw) To: Daniel P. Berrangé Cc: qemu-devel, Alex Benné e, Philippe Mathieu-Daudé , Peter Maydell, Paolo Bonzini, Stefan Hajnoczi 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-01-30 10:22 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-01-30 7:56 [PATCH v1] scripts/checkpatch.pl: check for placeholders in cover letter patches Manos Pitsidianakis 2024-01-30 8:12 ` Daniel P. Berrangé 2024-01-30 8:51 ` Manos Pitsidianakis 2024-01-30 9:47 ` Daniel P. Berrangé 2024-01-30 9:54 ` Manos Pitsidianakis 2024-01-30 9:59 ` Daniel P. Berrangé 2024-01-30 10:17 ` Manos Pitsidianakis
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).