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

[Bjorn Helgaas <helgaas@kernel.org>]

On Mon, Nov 07, 2022 at 01:00:59PM -0800, Peter Collingbourne wrote:
> On Mon, Nov 7, 2022 at 12:54 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > On Thu, Nov 03, 2022 at 06:34:31PM -0700, Peter Collingbourne wrote:
> > > On Thu, Nov 3, 2022 at 6:27 PM Joe Perches <joe@perches.com> wrote:
> > > > On Thu, 2022-11-03 at 18:07 -0700, Peter Collingbourne wrote:
> > > > > On Wed, Oct 19, 2022 at 03:28:43PM -0500, 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.
> > > > >
> > > > > If the commit message contains a line like:
> > > > >
> > > > > Cc: linux-arm-kernel@lists.infradead.org
> > > > >
> > > > > this patch causes checkpatch.pl to complain. Would it be possible to
> > > > > restrict this to URLs?
> > > >
> > > > Yes, I believe this would probably work well enough:
> > > > ---
> > > >  scripts/checkpatch.pl | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > > index 7be93c3df2bcb..fe25642d8bacc 100755
> > > > --- a/scripts/checkpatch.pl
> > > > +++ b/scripts/checkpatch.pl
> > > > @@ -3336,7 +3336,8 @@ sub process {
> > > >                 }
> > > >
> > > >  # Check for mailing list archives other than lore.kernel.org
> > > > -               if ($rawline =~ m{\b$obsolete_archives}) {
> > > > +               if ($rawline =~ m{\b$obsolete_archives} &&
> > > > +                   $rawline !~ /^\s*cc:/i) {
> > >
> > > Can we make this (to|cc): instead? Otherwise developers (like me) who
> > > use custom scripts to add To: headers to their patches before passing
> > > them to checkpatch.pl will also hit this warning if their patch is
> > > being sent To: one of these mailing lists.
> >
> > Why not make it look for "http" instead of the absence of "cc"?
> 
> "https" as well, but yes, that would make more sense to me, and would
> be less likely to require user workarounds.

Maybe like this?  (On top of my previous attempt, which is in -next)


commit d15f85247948 ("checkpatch: warn only for URLs to non-lore archives")
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Mon Nov 14 16:33:12 2022 -0600

    checkpatch: warn only for URLs to non-lore archives
    
    Previously we warned for anything that contained the archive hostname, but
    some email addresses also contain those hostnames, and we'd rather not warn
    about those.  Only warn if we see "http" before the archive hostname.
    
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    ---
    
    Sample commit for testing with "checkpatch -g":
    
      5e91e57e6809 Cc: linux-arm-kernel@lists.infradead.org

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1c3d13e65c2d..78cc595b98ce 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3336,7 +3336,7 @@ sub process {
 		}
 
 # Check for mailing list archives other than lore.kernel.org
-		if ($rawline =~ m{\b$obsolete_archives}) {
+		if ($rawline =~ m{http.*\b$obsolete_archives}) {
 			WARN("PREFER_LORE_ARCHIVE",
 			     "Use lore.kernel.org archive links when possible - see https://lore.kernel.org/lists.html\n" . $herecurr);
 		}