Re: [PATCH] checkpatch: add warning for non-lore mailing list URLs

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Feb 10, 2021 at 12:22:35AM -0800, Kees Cook wrote:
> On Thu, Dec 17, 2020 at 04:50:41PM -0800, Joe Perches wrote:
> > On Thu, 2020-12-17 at 17:56 -0600, Bjorn Helgaas wrote:
> > > From: Bjorn Helgaas <bhelgaas@google.com>
> > > 
> > > The lkml.org, marc.info, spinics.net, etc archives are not quite as useful
> > > as lore.kernel.org because they use different styles, add advertising, and
> > > may disappear in the future.  The lore archives are more consistent and
> > > more likely to stick around, so prefer https://lore.kernel.org URLs when
> > > they exist.
> > 
> > Hi Bjorn.
> > 
> > I like the idea, thanks, but a couple notes.
> > 
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > []
> > > @@ -564,6 +564,17 @@ sub find_standard_signature {
> > >  	return "";
> > >  }
> >  
> > > +our $obsolete_archives = qr{(?xi:
> > > +	freedesktop.org/archives/dri-devel|
> > > +	lists.infradead.org|
> > > +	lkml.org|
> > > +	mail-archive.com|
> > > +	mailman.alsa-project.org/pipermail|
> > > +	marc.info|
> > > +	ozlabs.org/pipermail|
> > > +	spinics.net
> > > +)};
> > 
> > Strictly, these all need \Q \E escaping so uses like lkmlAorg do not match.
> > 
> > 
> > > @@ -3101,6 +3112,12 @@ sub process {
> > >  			}
> > >  		}
> > >  
> > > +# Check for mailing list archives other than lore.kernel.org
> > > +		if ($line =~ /(http|https):\/\/\S*$obsolete_archives/) {
> > 
> > The https?:// doesn't seem necessary.  Perhaps:
> > 
> > 		if ($line =~ m{\b$obsolete_archives}) {
> > 
> > > +			WARN("PREFER_LORE_ARCHIVE",
> > > +			     "Use lore.kernel.org archive links when possible; see https://lore.kernel.org/lists.html\n" . $herecurr);
> > 
> > Perhaps:
> > 			     "Prefer lore.kernel.org links. see: https://www.kernel.org/lore.html#linking-to-list-discussions-from-commits\n" . $herecurr);
> > 
> > So maybe instead:
> > ---
> >  scripts/checkpatch.pl | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 00085308ed9d..c2a324d628a6 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -564,6 +564,17 @@ sub find_standard_signature {
> >  	return "";
> >  }
> >  
> > +our $obsolete_archives = qr{(?xi:
> > +	\Qfreedesktop.org/archives/dri-devel\E |
> > +	\Qlists.infradead.org\E |
> > +	\Qlkml.org\E |
> > +	\Qmail-archive.com\E |
> > +	\Qmailman.alsa-project.org/pipermail\E |
> > +	\Qmarc.info\E |
> > +	\Qozlabs.org/pipermail\E |
> > +	\Qspinics.net\E
> > +)};
> > +
> >  our @typeListMisordered = (
> >  	qr{char\s+(?:un)?signed},
> >  	qr{int\s+(?:(?:un)?signed\s+)?short\s},
> > @@ -3101,6 +3112,12 @@ sub process {
> >  			}
> >  		}
> >  
> > +		# Check for mailing list archives other than lore.kernel.org
> > +		if ($rawline =~ m{\b$obsolete_archives}) {
> > +			WARN("PREFER_LORE_ARCHIVE",
> > +			     "Use lore.kernel.org archive links when possible - see https://lore.kernel.org/lists.html\n" . $herecurr);
> > +		}
> > +
> >  # Check for added, moved or deleted files
> >  		if (!$reported_maintainer_file && !$in_commit_log &&
> >  		    ($line =~ /^(?:new|deleted) file mode\s*\d+\s*$/ ||
> > 
> > 
> 
> Ah, nice. Yes, this would be great to get added. Joe, can you respin as
> a full path? Please consider it:

I hate to ask Joe to rework *my* patch just because I've dropped the
ball on it!  Sorry, I'll try to resurrect this.

> Reviewed-by: Kees Cook <keescook@chromium.org>