Message ID | 20221019202843.40810-1-helgaas@kernel.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4ac7:0:0:0:0:0 with SMTP id y7csp525047wrs; Wed, 19 Oct 2022 13:40:51 -0700 (PDT) X-Google-Smtp-Source: AMsMyM4zeqaTpI9QD72UszqAXCR0yBsEvgv8eCkxReusTiQ6wHUOPdxkmCBTD3Rje99YrZnkiRDc X-Received: by 2002:a63:188:0:b0:43c:22e9:2d10 with SMTP id 130-20020a630188000000b0043c22e92d10mr8798684pgb.12.1666212051312; Wed, 19 Oct 2022 13:40:51 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1666212051; cv=none; d=google.com; s=arc-20160816; b=PocfU+u41PrajGh2EZ8TYajd3oTs5pqjRF0FE9HZegjrttODIfdCHxgwgHZFB5yMe1 CllCZRzckPt/qeLKs/xMFSzxBHKV2/tC0RQjJ9tm2Bp48jdDWxzOJedgiJJFERh7/FEz TeoCoRzviBBeYAEJ8xcR6PkA20fAjqOwbl+BU5iVoy1VAYvcZaBB1PI7qBO4O3SHqLjA H8AjM1CCCKAbgB3y/ItM4cqgoDOtgUMwqiV9bDmC6Q5zFk6qS84mp36TOC55Min0IpD0 delLWFEsWT5Kge2/A6ylO1FNHNg1QFovtB2jVrgGjbm8cZEX9W7wYgefxLCIANcxibpL PK7g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=hPn0td1Q55uzAy5lTjCPcPVyy01hPtfNZTMMIPuU8hc=; b=0SId7lzEXCYOjfYGgoTrquJNn4pQOGyPaFPWach+t6EqNPpusCueYQaS53ThFO5hcT 5JPFZYp9syYC9yUEBon33CLC4hHr7bjwIWp7xBgDm/yR0eYfcBWuJkmt8JOcIXhgO1xd uhvPkAaB1NbmN0TEQ7ScX/Zgv6prsrG/we3iFm1Q/4R4lIrUAo6Mv1PeGDXsc9FdkXs+ +vbvY/gDp7U1YTmIu++uk+pNBhYLcaBg5ZUIB93aLIYz8DP0BFeyRaYCSD4h4WNNEmnT QfQ8g9QlY0VVfRJRGxM3xukuayy75ZnfpBwHgXEbhQbaYlusitZGH9nOdwGyIVZsMwfx gF1g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=Ys10kToS; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id e7-20020a170902b78700b001733f3e79e4si17161624pls.607.2022.10.19.13.40.37; Wed, 19 Oct 2022 13:40:51 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=Ys10kToS; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230218AbiJSU3D (ORCPT <rfc822;samuel.l.nystrom@gmail.com> + 99 others); Wed, 19 Oct 2022 16:29:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40800 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231374AbiJSU2y (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 19 Oct 2022 16:28:54 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EAAFC1C8106 for <linux-kernel@vger.kernel.org>; Wed, 19 Oct 2022 13:28:52 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 907F5619C2 for <linux-kernel@vger.kernel.org>; Wed, 19 Oct 2022 20:28:52 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BC9F7C433D6; Wed, 19 Oct 2022 20:28:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1666211331; bh=u83e/tAeEj/i+VC3hmO8X6CBG1Ga+kJc8l/dPOEqfKI=; h=From:To:Cc:Subject:Date:From; b=Ys10kToSr0KK7GNR5ge5kSEIIrV7ZkV+/vgIC6dtbhfAIzFwkjQV+loQuZFu1Bl9Y j+6V0d3MFg9iYQ35eXsWxATV+Wdvx2GdJWG/WHMgq+dZ/DpQqNkWwURRKE1yKfYwvi 5ePv9snJK8beFvMLJpQU8ycYqSIhT+wkJZmTMnstTeMJbDR9zFPDHHDjorBikogaPg Ms8jDc6oPc5VN9v+wRR+6keUe/icOJYIu4EzMedju+Dxv/7xIOWCh6miCFQW47apgj OQa/Kfrrsp6VDqrmt/vFSsiXGcPDoR8brZA1DY5nCNG64gTLFypQgCi5qjq/1XHOpq ILF0jxC50bD/w== From: Bjorn Helgaas <helgaas@kernel.org> To: Andy Whitcroft <apw@canonical.com>, Joe Perches <joe@perches.com> Cc: Dwaipayan Ray <dwaipayanray1@gmail.com>, Lukas Bulwahn <lukas.bulwahn@gmail.com>, Kees Cook <keescook@chromium.org>, Randy Dunlap <rdunlap@infradead.org>, linux-kernel@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com> Subject: [PATCH] checkpatch: add warning for non-lore mailing list URLs Date: Wed, 19 Oct 2022 15:28:43 -0500 Message-Id: <20221019202843.40810-1-helgaas@kernel.org> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1747149967674520315?= X-GMAIL-MSGID: =?utf-8?q?1747149967674520315?= |
Series |
checkpatch: add warning for non-lore mailing list URLs
|
|
Commit Message
Bjorn Helgaas
Oct. 19, 2022, 8:28 p.m. UTC
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. Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> --- Sample commits for testing with "checkpatch -g": bd82d4bd2188 www.spinics.net/lists/arm-kernel/msg716956.html fdec2a9ef853 www.spinics.net/lists/kvm-arm 1cdca16c043a www.spinics.net/lists/linux-mmc 48ea02184a9d www.spinics.net/lists/linux-pci f32ae8a5f131 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 www.mail-archive.com/linux-kernel@vger.kernel.org 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/20201217235615.43328-1-helgaas@kernel.org/ https://lore.kernel.org/all/20220401201417.126664-1-helgaas@kernel.org/ --- scripts/checkpatch.pl | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
Comments
On Wed, 2022-10-19 at 15:28 -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. > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> Seems sensible, thanks. > --- > > Sample commits for testing with "checkpatch -g": > > bd82d4bd2188 www.spinics.net/lists/arm-kernel/msg716956.html > fdec2a9ef853 www.spinics.net/lists/kvm-arm > 1cdca16c043a www.spinics.net/lists/linux-mmc > 48ea02184a9d www.spinics.net/lists/linux-pci > f32ae8a5f131 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 www.mail-archive.com/linux-kernel@vger.kernel.org > 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/20201217235615.43328-1-helgaas@kernel.org/ > https://lore.kernel.org/all/20220401201417.126664-1-helgaas@kernel.org/ > --- > 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*$/ ||
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? Peter
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) { WARN("PREFER_LORE_ARCHIVE", "Use lore.kernel.org archive links when possible - see https://lore.kernel.org/lists.html\n" . $herecurr); }
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. Peter
On Thu, 2022-11-03 at 18:34 -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. 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.
On Thu, Nov 3, 2022 at 6:41 PM Joe Perches <joe@perches.com> wrote: > > On Thu, 2022-11-03 at 18:34 -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. > > 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
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"?
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. Peter
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); }
On Mon, Nov 14, 2022 at 2:43 PM Bjorn Helgaas <helgaas@kernel.org> 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 <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> Reviewed-by: Peter Collingbourne <pcc@google.com> Peter
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*$/ ||