2022-10-19 21:31:27

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH] checkpatch: add warning for non-lore mailing list URLs

From: Bjorn Helgaas <[email protected]>

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.

Signed-off-by: Bjorn Helgaas <[email protected]>
---

Sample commits for testing with "checkpatch -g":

bd82d4bd2188 http://www.spinics.net/lists/arm-kernel/msg716956.html
fdec2a9ef853 http://www.spinics.net/lists/kvm-arm
1cdca16c043a http://www.spinics.net/lists/linux-mmc
48ea02184a9d http://www.spinics.net/lists/linux-pci
f32ae8a5f131 http://www.spinics.net/lists/netdev
b7dca6dd1e59 lkml.org
265df32eae58 lkml.org/lkml/
4a9ceb7dbadf marc.info/?l=linux-kernel&m=155656897409107&w=2.
c03914b7aa31 marc.info/?l=linux-mm
f108c887d089 marc.info/?l=linux-netdev
7424edbb5590 marc.info/?t=156200975600004&r=1&w=2
dabac6e460ce https://marc.info/?l=linux-rdma&m=152296522708522&w=2
b02f6a2ef0a1 http://www.mail-archive.com/[email protected]
5e91bf5ce9b8 lists.infradead.org/pipermail/linux-snps-arc/2019-May
3cde818cd02b mailman.alsa-project.org/pipermail/alsa-devel/2019-January/144761.html
a5448fdc469d http://lists.infradead.org/pipermail/linux-nvme/2019-June/024721.html

Previously posted:
https://lore.kernel.org/all/[email protected]/
https://lore.kernel.org/all/[email protected]/
---
scripts/checkpatch.pl | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1e5e66ae5a52..4e187202e77a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -702,6 +702,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},
@@ -3324,6 +3335,12 @@ sub process {
$last_git_commit_id_linenr = $linenr if ($line =~ /\bcommit\s*$/i);
}

+# 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*$/ ||
--
2.25.1


2022-10-19 21:36:04

by Joe Perches

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

On Wed, 2022-10-19 at 15:28 -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <[email protected]>
>
> 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.
>
> Signed-off-by: Bjorn Helgaas <[email protected]>

Seems sensible, thanks.

> ---
>
> Sample commits for testing with "checkpatch -g":
>
> bd82d4bd2188 http://www.spinics.net/lists/arm-kernel/msg716956.html
> fdec2a9ef853 http://www.spinics.net/lists/kvm-arm
> 1cdca16c043a http://www.spinics.net/lists/linux-mmc
> 48ea02184a9d http://www.spinics.net/lists/linux-pci
> f32ae8a5f131 http://www.spinics.net/lists/netdev
> b7dca6dd1e59 lkml.org
> 265df32eae58 lkml.org/lkml/
> 4a9ceb7dbadf marc.info/?l=linux-kernel&m=155656897409107&w=2.
> c03914b7aa31 marc.info/?l=linux-mm
> f108c887d089 marc.info/?l=linux-netdev
> 7424edbb5590 marc.info/?t=156200975600004&r=1&w=2
> dabac6e460ce https://marc.info/?l=linux-rdma&m=152296522708522&w=2
> b02f6a2ef0a1 http://www.mail-archive.com/[email protected]
> 5e91bf5ce9b8 lists.infradead.org/pipermail/linux-snps-arc/2019-May
> 3cde818cd02b mailman.alsa-project.org/pipermail/alsa-devel/2019-January/144761.html
> a5448fdc469d http://lists.infradead.org/pipermail/linux-nvme/2019-June/024721.html
>
> Previously posted:
> https://lore.kernel.org/all/[email protected]/
> https://lore.kernel.org/all/[email protected]/
> ---
> scripts/checkpatch.pl | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 1e5e66ae5a52..4e187202e77a 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -702,6 +702,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},
> @@ -3324,6 +3335,12 @@ sub process {
> $last_git_commit_id_linenr = $linenr if ($line =~ /\bcommit\s*$/i);
> }
>
> +# 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*$/ ||

2022-11-04 01:53:35

by Joe Perches

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

On Thu, 2022-11-03 at 18:34 -0700, Peter Collingbourne wrote:
> On Thu, Nov 3, 2022 at 6:27 PM Joe Perches <[email protected]> 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 <[email protected]>
> > > >
> > > > 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: [email protected]
> > >
> > > 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.

I think adding "To:" would be odd and unnecessary as it's not
something that would actually be in a patch.

You could use another front-end script to strip those "To:" from
checkpatch inputs.


2022-11-04 02:07:50

by Peter Collingbourne

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

On Wed, Oct 19, 2022 at 03:28:43PM -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <[email protected]>
>
> 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: [email protected]

this patch causes checkpatch.pl to complain. Would it be possible to
restrict this to URLs?

Peter

2022-11-04 02:08:36

by Joe Perches

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

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 <[email protected]>
> >
> > 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: [email protected]
>
> 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) {
WARN("PREFER_LORE_ARCHIVE",
"Use lore.kernel.org archive links when possible - see https://lore.kernel.org/lists.html\n" . $herecurr);
}


2022-11-04 02:45:46

by Peter Collingbourne

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

On Thu, Nov 3, 2022 at 6:27 PM Joe Perches <[email protected]> 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 <[email protected]>
> > >
> > > 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: [email protected]
> >
> > 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.

Peter

2022-11-04 17:03:50

by Peter Collingbourne

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

On Thu, Nov 3, 2022 at 6:41 PM Joe Perches <[email protected]> wrote:
>
> On Thu, 2022-11-03 at 18:34 -0700, Peter Collingbourne wrote:
> > On Thu, Nov 3, 2022 at 6:27 PM Joe Perches <[email protected]> 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 <[email protected]>
> > > > >
> > > > > 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: [email protected]
> > > >
> > > > 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.
>
> I think adding "To:" would be odd and unnecessary as it's not
> something that would actually be in a patch.
>
> You could use another front-end script to strip those "To:" from
> checkpatch inputs.

OK, I made that work, so I guess I don't mind much what we do here.

Peter

2022-11-07 21:54:23

by Peter Collingbourne

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

On Mon, Nov 7, 2022 at 12:54 PM Bjorn Helgaas <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > > >
> > > > > 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: [email protected]
> > > >
> > > > 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.

Peter

2022-11-07 21:58:34

by Bjorn Helgaas

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

On Thu, Nov 03, 2022 at 06:34:31PM -0700, Peter Collingbourne wrote:
> On Thu, Nov 3, 2022 at 6:27 PM Joe Perches <[email protected]> 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 <[email protected]>
> > > >
> > > > 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: [email protected]
> > >
> > > 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"?

2022-11-14 23:04:54

by Peter Collingbourne

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

On Mon, Nov 14, 2022 at 2:43 PM Bjorn Helgaas <[email protected]> wrote:
>
> On Mon, Nov 07, 2022 at 01:00:59PM -0800, Peter Collingbourne wrote:
> > On Mon, Nov 7, 2022 at 12:54 PM Bjorn Helgaas <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > > > > >
> > > > > > > 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: [email protected]
> > > > > >
> > > > > > 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 <[email protected]>
> 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 <[email protected]>

Reviewed-by: Peter Collingbourne <[email protected]>

Peter

2022-11-15 00:04:14

by Bjorn Helgaas

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

On Mon, Nov 07, 2022 at 01:00:59PM -0800, Peter Collingbourne wrote:
> On Mon, Nov 7, 2022 at 12:54 PM Bjorn Helgaas <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > > > >
> > > > > > 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: [email protected]
> > > > >
> > > > > 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 <[email protected]>
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 <[email protected]>
---

Sample commit for testing with "checkpatch -g":

5e91e57e6809 Cc: [email protected]

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);
}