Re: Fw: [PATCH 0/2] feat: checkpatch: prohibit Buglink: and warn about missing Link:

From: Joe Perches
Date: Tue Dec 06 2022 - 00:54:40 EST


On Tue, 2022-12-06 at 05:55 +0100, Thorsten Leemhuis wrote:
> Hi Joe! Many thx for looking into this.
>
> 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 <kai@xxxxxxxxxxxxxxxxxxxxxx>
> > > To: linux-kernel@xxxxxxxxxxxxxxx
> > > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>, Thorsten Leemhuis <linux@xxxxxxxxxxxxx>
> > > 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 <linux@xxxxxxxxxxxxx>
> > > Signed-off-by: Kai Wasserbäch <kai@xxxxxxxxxxxxxxxxxxxxxx>
> > >
> > > 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:
>
> ```
> > > BugLink: https://lore.kernel.org/r/20220610205038.GA3050413@paulmck-ThinkPad-P17-Gen-1
> > > BugLink: https://lore.kernel.org/r/CAMdYzYpF4FNTBPZsEFeWRuEwSies36QM_As8osPWZSr2q-viEA@xxxxxxxxxxxxxx
> > [...]
> > please stop making up random tags that make no sense.
> >
> > Just use "Link:"
> > [...]
> >
> > It's extra context to the commit, in case somebody wants to know the
> > history. The "bug" part is (and always should be) already explained in
> > the commit message, there's absolutely no point in adding soem extra
> > noise to the "Link:" tag.>
> ```
> That's a quote from
> https://lore.kernel.org/all/CAHk-=wgs38ZrfPvy=nOwVkVzjpM3VFU1zobP37Fwd_h9iAD5JQ@xxxxxxxxxxxxxx/
>
> In that message he also links to another mail from him, where he wrote:
> ```
> > I think "BugLink:" is silly, because that's just a regular link.
> > What's the upside of saying "Bug" in there? If you're fixing a bug,
> > and are linking to reports and discussions by people, what does that
> > "bug" buy you apart from another ugly CamelCase thing?
> >
> > In other words, in "BugLink:", neither "Bug" nor "Link" is actually meaningful.
> >
> > The current "Link:" tag exists AS A GENERIC WAY TO SAY "look, here's
> > more information". That's the point. The word "Link" has no other
> > meaning, and trying to then combine it with other things is worthless.
> ```
> https://lore.kernel.org/all/CAHk-=wgzRUT1fBpuz3xcN+YdsX0SxqOzHWRtj0ReHpUBb5TKbA@xxxxxxxxxxxxxx/
>
> Our docs say to use Link in this case, too; see
> Documentation/process/submitting-patches.rst
> (http://docs.kernel.org/process/submitting-patches.html) and
> Documentation/process/5.Posting.rst
> (https://docs.kernel.org/process/5.Posting.html)
>
> And using various tags for the same thing makes it also harder for
> external scripts that look at commits to take further action -- like the
> regression tracking bot I write and use for my work.
>
> 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 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.

> > > 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.