Re: [PATCH 2/2] checkpatch: warn when Reported-by: is not followed by Link:

[Joe Perches <joe@perches.com>]

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