Re: [PATCH] checkpatch: Add check for common mailing list helper checks

From: Joe Perches
Date: Wed Jul 07 2021 - 13:53:54 EST


On Tue, 2021-07-06 at 12:31 -0700, John 'Warthog9' Hawley wrote:
> On 7/3/21 11:39 AM, Joe Perches wrote:
> > On Fri, 2021-07-02 at 15:37 -0700, John 'Warthog9' Hawley (VMware)
> > wrote:
> > > From: John 'Warthog9' Hawley <warthog9@xxxxxxxxxxxxxx>
> > >
> > > Mailing lists in an attempt to try and avoid sending certain
> > > administrative e-mails to the list, will check the first few lines
> > > (usually ~10) looking for keywords. If those key words are found it
> > > shunts the e-mail to the list admin contact instead of potentially
> > > passing it through to the list.
> >
> > Perhaps the below is a bit better, but I believe a few of the tests
> > are going to be tripped a bit too often.
> >
> > Especially "cancel", "config" and maybe "subscribe" too.
> >
> > For instance:
> >
> > $ git log --grep='\bcancel\b' -P -i --pretty=oneline -10000 | wc -l
> > 1693
> >
> > $ git log --grep='^config\b' -P -i --pretty=oneline -10000 | wc -l
> > 890
> >
> > $ git log --grep='\bsubscribe\b' -P -i --pretty=oneline -10000 | wc -l
> > 123
>
> A part of getting this into checkpatch.pl is getting some better
> feedback mechanisms for why patches may not be passing through the list
> correctly with regexes that have been in place for at least 14 years.
> These, aren't tripped over often,

3000+ commits with regex matches seem rather a lot to me.

> but have run into a instance at least
> recently that triggered me trying to get at least some self check, and
> notification, pieces in place.

No worries, but perhaps the message might be reworded to
say something about possible mailing list moderation rather
than imply rejection.

> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> > @@ -865,6 +865,37 @@ our $allowed_asm_includes = qr{(?x:
> >  )};
> >  # memory.h: ARM has a custom one
> >
> > +our $mailing_list_phrases = qr{(?xi:
> > + \bcancel\b |

Mere use of the word "cancel" in the commit description seems undesirable to me.

> > +# check if words in the commit message may trip up common mailing list helpers
> > +# to redirect email to the admin contact
> > + if ($in_commit_log && $commit_log_lines < 10 &&
> > + $line =~ /($mailing_list_phrases)/) {
> > + WARN("MAILING_LIST_HELPER",
> > + "Line matches common mailing list helpers and may not be delivered correctly - consider rewording '$1'\n" . $herecurr);

Maybe FILTERS for phrases and helpers

Maybe something like:

"Use of '$1' in this patch's commit description might cause mailing list moderation or rejection\n"