From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1C1C0C3A5A7 for ; Tue, 6 Dec 2022 07:17:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233169AbiLFHRp (ORCPT ); Tue, 6 Dec 2022 02:17:45 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51518 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230348AbiLFHRk (ORCPT ); Tue, 6 Dec 2022 02:17:40 -0500 Received: from wp530.webpack.hosteurope.de (wp530.webpack.hosteurope.de [80.237.130.52]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CE506BAE for ; Mon, 5 Dec 2022 23:17:37 -0800 (PST) Received: from [2a02:8108:963f:de38:eca4:7d19:f9a2:22c5]; authenticated by wp530.webpack.hosteurope.de running ExIM with esmtpsa (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) id 1p2SCt-00068p-WD; Tue, 06 Dec 2022 08:17:36 +0100 Message-ID: Date: Tue, 6 Dec 2022 08:17:35 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.5.1 Content-Language: en-US, de-DE From: Thorsten Leemhuis To: Joe Perches Cc: LKML , Andrew Morton , =?UTF-8?Q?Kai_Wasserb=c3=a4ch?= References: <20221205131424.36909375d90d5a40cd028bc0@linux-foundation.org> <11a9fe60f5333a931b8d75f67808b6d923c16dfa.camel@perches.com> <25f4838b-208a-cf8c-914c-b2092665d56f@leemhuis.info> <23a61dd072ee1d2cc5b54281b0a9dc13e01aa0b8.camel@perches.com> Subject: Re: Fw: [PATCH 0/2] feat: checkpatch: prohibit Buglink: and warn about missing Link: In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-bounce-key: webpack.hosteurope.de;linux@leemhuis.info;1670311057;e011be4a; X-HE-SMSGID: 1p2SCt-00068p-WD Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06.12.22 07:27, Thorsten Leemhuis wrote: > On 06.12.22 06:54, Joe Perches wrote: >> On Tue, 2022-12-06 at 05:55 +0100, Thorsten Leemhuis wrote: >>> On 06.12.22 02:02, Joe Perches wrote: >>>> On Mon, 2022-12-05 at 13:14 -0800, Andrew Morton wrote: >>>>> Begin forwarded message: >>>>> >>>>> Date: Sun, 4 Dec 2022 12:33:37 +0100 >>>>> From: Kai Wasserbäch >>>>> To: linux-kernel@vger.kernel.org >>>>> Cc: Andrew Morton , Thorsten Leemhuis >>>>> Subject: [PATCH 0/2] feat: checkpatch: prohibit Buglink: and warn about missing Link: >>>>> >>>>> Thorsten Leemhuis suggested the following two changes to checkpatch, which >>>>> I hereby humbly submit for review. Please let me know if any changes >>>>> would be required for acceptance. >>>> [...] >>>>> Suggested-by: Thorsten Leemhuis >>>>> Signed-off-by: Kai Wasserbäch >>>>> >>>>> Kai Wasserbäch (2): >>>>> feat: checkpatch: error on usage of a Buglink tag in the commit log >>>> >>>> Why, what's wrong with a buglink reference? >>> >>> Long story short: Linus doesn't like it: >> >> [...] >> >>> All of that obviously should have been in patch description. Let me >>> resubmit that patch with all of that in there, or are you dead against >>> this idea now? >> >> No, better commit description would be useful > > I'll prepare something. > >> and perhaps a more >> generic, "is the thing in front of a URI/URL" a known/supported entry, >> instead of using an known invalid test would be a better mechanism. > > Are you sure about that? It's not that I disagree completely, but it > sounds overly restrictive to me and makes it harder for new tags to > evolve in case we might want them. > > And what tags would be on this allow-list? Anything else then "Link" and > "Patchwork"? Those are the ones that looked common and valid to me when > I ran > > git log --grep='http' v4.0.. | grep http | grep -v ' Link: ' | less > > and skimmed the output. Maybe "Datasheet" should be allowed, too -- not > sure. > > But I found a few others that likely should be on the disallow list: > "Closes:", "Bug:", "Gitlab issue:", "References:", "Ref:", "Bugzilla:", > "RHBZ:", and "link", as "Link" should be used instead in all of these > cases afaics. > >>>>> feat: checkpatch: Warn about Reported-by: not being followed by a >>>>> Link: >>>> >>>> I think this is unnecessary. >>> >>> I expect this to cause more discussion, which is why this should be in a >>> separate submission. But in the end the reasons are similar as above: >>> (1) Linus really want to see those links to make things easier for >>> future code archeologists. (2) My regression tracking efforts heavily >>> rely on those Links, as they allow to automatically connect reports with >>> patches and commits to fix the reported regression; without those the >>> tracking is a PITA and doesn't scale. >>> >>> Together that IMHO is strong enough reason to warn in this case. >> >> Maybe, I think there might be some value in not intermixing signature >> tags and other tags though. > > Hmm, sorry, not sure if I understood you here. Why do you consider > "Reported-by:" a signature tag? Isn't it more of an "appreciation" tag, > IOW, a kind of 'thank you' hat tip to the reporter? Sorry, now I understand[1]: you consider "Reported-by:" a signature tag as it's used in the area where the Signed-off-by resist. Well, yeah, maybe it shouldn't be like that[2]. But it's quite normal afaics to use Reported-by: (with and without Link:) in that area currently. Changing that would break habits of many kernel developers. And having the Reported-by: and the Link: next to each other IMHO is important, as it makes it obvious what the Link is about; that's afaics also why Linus wants it that way, as can be seen from the quotes I provided earlier. See also commit 0ba09b173387 from a few days ago[3], where he afaics added the Link tags to reports of that regression himself. Ciao, Thorsten [1] Kai (many thx!) explained and pointed me to the area in the checkpatch.pl code that made it obvious: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/checkpatch.pl#n611 [2] in an ideal world things like that and tags like Tested-by: would resist in git notes or something, as then they could be extended after a change was committed... [3] https://git.kernel.org/torvalds/c/0ba09b173387