Messages in this thread | | | Subject | Re: [PATCH 2/2] checkpatch: warn when Reported-by: is not followed by Link: | From | Joe Perches <> | Date | Thu, 08 Dec 2022 13:34:18 -0800 |
| |
On Thu, 2022-12-08 at 22:11 +0100, Thorsten Leemhuis wrote: > Joe, many thx for your time and your valuable feedback. I agree with > most of it and will look into addressing it tomorrow, but there is one > area where I have a different option: > > On 08.12.22 21:21, Joe Perches wrote: > > On Thu, 2022-12-08 at 20:32 +0100, Kai Wasserbäch wrote: > > > + > > > + # check if Reported-by: is followed by a Link: > > > + if ($sign_off =~ /^reported-by:$/i) { > > > + if (!defined $lines[$linenr]) { > > > + WARN("BAD_REPORTED_BY_LINK", > > > + "Reported-by: should be immediately followed by Link: to the report\n" . "$here\n" . $rawline); > > > + } elsif ($rawlines[$linenr] !~ /^\s*link:\s*(.*)/i) { > > > + WARN("BAD_REPORTED_BY_LINK", > > > + "Reported-by: should be immediately followed by Link: to the report\n" . "$here\n" . $rawline . "\n" .$rawlines[$linenr]); > > > + } elsif ($lines[$linenr] !~ /https?:\/\//i) { > > > + WARN("BAD_REPORTED_BY_LINK", > > > + "Link: following Reported-by: should contain an URL\n" . "$here\n" . $rawline . "\n" .$rawlines[$linenr]); > > > > [...] > > > > Also the actual link line should likely be from lore > > > > So maybe: > > } elsif ($lines[$linenr] !~ m{https?://lore.kernel.org/.+}i) { > > WARN("BAD_REPORTED_BY_LINK", > > "Link: following Reported-by: should use a lore.kernel.org URL\n" . $herecurr . $rawlines[$linenr]); > > I'm pretty sure that's not want we want, as from regression tracking I > known that we have other links that should continue to work here. Links > to bugzilla.kernel.org immediately come to my mind, for example. Then > there are some subsystems that use issue trackers on github or in gitlab > instances (DRM for example), which must work here, too; and we > occasionally even have links to bugs in bugzilla instances from RH or > SUSE there, as sometimes valuable details for code archeologists can be > found there.
Outside the fact there are relatively few existing uses of Link: after Reported-by:, it appears that "Fixes:" should also be allowed.
$ git log --no-merges --format=email -P -i --grep "^Reported-by:" -100000 | \ grep -A1 "^Reported-by:" | \ grep -v -P '^(Reported-by:|\-\-)' | \ cut -f1 -d":" | sort | uniq -c | sort -rn | head -20 31719 Signed-off-by 4556 Tested-by 4276 Cc 2976 Fixes 2304 Reviewed-by 1526 Acked-by 949 Suggested-by 821 Link 387 228 CC 139 Bugzilla 82 Reported-and-tested-by 69 Debugged-by 68 References 63 Bisected-by 43 Co-developed-by 34 LKML-Reference 29 Diagnosed-by 27 [ paulmck 26 Addresses-Coverity-ID
and lore definitely dominates the Link: uses.
And IMO: the lkml.kernel.org entries should instead use lore.
Maybe there should be some other patterns allowed. Dunno.
$ git log --no-merges --format=email -P -i --grep "^Reported-by:" -100000 | \ grep -A1 "^Reported-by:" | \ grep '^Link:' | \ cut -f1-3 -d"/" | sort | uniq -c | sort -rn | head -20 551 Link: https://lore.kernel.org 67 Link: http://lkml.kernel.org 42 Link: https://bugzilla.kernel.org 28 Link: https://github.com 16 Link: https://lkml.kernel.org 11 Link: lore.kernel.org/r/64644a2f-4a20-bab3-1e15-3b2cdd0defe3@omprussia.ru 10 Link: https://patchwork.freedesktop.org 10 Link: https://github.blog 9 Link: https://syzkaller.appspot.com 9 Link: https://lkml.org 6 Link: https://www.spinics.net 5 Link: https://gitlab.freedesktop.org 5 Link: https://bugs.debian.org 5 Link: http://lore.kernel.org 4 Link: https://www.openwall.com 4 Link: https://patchwork.kernel.org 4 Link: http://patchwork.freedesktop.org 3 Link: https://marc.info 3 Link: https://bugzilla.linux-nfs.org 2 Link: https://bugzilla.suse.com
| |