All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/5] docs & checkpatch: allow Closes tags with links
@ 2023-04-03 16:23 ` Matthieu Baerts
  0 siblings, 0 replies; 19+ messages in thread
From: Matthieu Baerts @ 2023-04-03 16:23 UTC (permalink / raw)
  To: Jonathan Corbet, Andy Whitcroft, Joe Perches, Dwaipayan Ray,
	Lukas Bulwahn, Kai Wasserbäch, Thorsten Leemhuis,
	Andrew Morton, David Airlie, Daniel Vetter, Konstantin Ryabitsev,
	Bagas Sanjaya, Linus Torvalds
  Cc: linux-doc, linux-kernel, dri-devel, mptcp, Matthieu Baerts

Since v6.3, checkpatch.pl now complains about the use of "Closes:" tags
followed by a link [1]. It also complains if a "Reported-by:" tag is
followed by a "Closes:" one [2].

As detailed in the first patch, this "Closes:" tag is used for a bit of
time, mainly by DRM and MPTCP subsystems. It is used by some bug
trackers to automate the closure of issues when a patch is accepted.
It is even planned to use this tag with bugzilla.kernel.org [3].

The first patch updates the documentation to explain what is this
"Closes:" tag and how/when to use it. The second patch modifies
checkpatch.pl to stop complaining about it.

The DRM maintainers and their mailing list have been added in Cc as they
are probably interested by these two patches as well.

[1] https://lore.kernel.org/all/3b036087d80b8c0e07a46a1dbaaf4ad0d018f8d5.1674217480.git.linux@leemhuis.info/
[2] https://lore.kernel.org/all/bb5dfd55ea2026303ab2296f4a6df3da7dd64006.1674217480.git.linux@leemhuis.info/
[3] https://lore.kernel.org/linux-doc/20230315181205.f3av7h6owqzzw64p@meerkat.local/

Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
Note: After having re-read the comments from the v1, it is still unclear
to me if this "Closes:" can be accepted or not. But because it seems
that the future Bugzilla bot for kernel.org and regzbot would like to
use it as well, I'm sending here new versions. I'm sorry if I
misunderstood the comments from v1. Please tell me if I did.

Changes in v4:
- Patches 1/5, 3/5 and 4/5 have been added to ask using the "Closes" tag
  instead of the "Link" one for any bug reports. (Thorsten)
- The Fixes tags have been removed from patch 4/5. (Joe)
- The "Reported-by being followed by a link tag" check is now only
  looking for the tag, not the URL which is done elsewhere in patch 5/5.
  (Thorsten)
- A new patch has been added to fix a small issues in checkpatch.pl when
  checking if "Reported-by:" tag is on the last line.
- Link to v3: https://lore.kernel.org/r/20230314-doc-checkpatch-closes-tag-v3-0-d1bdcf31c71c@tessares.net

Changes in v3:
- Patch 1/4 now allow using the "Closes" tag with any kind of bug
  reports, as long as the link is public. (Thorsten)
- The former patch 2/2 has been split in two: first to use a list for
  the different "link" tags (Joe). Then to allow the 'Closes' tag.
- A new patch has been added to let checkpatch.pl checking if "Closes"
  and "Links" are used with a URL.
- Link to v2: https://lore.kernel.org/r/20230314-doc-checkpatch-closes-tag-v2-0-f4a417861f6d@tessares.net

Changes in v2:
- The text on patch 1/2 has been reworked thanks to Jon, Bagas and
  Thorsten. See the individual changelog on the patch for more details.
- Private bug trackers and invalid URLs are clearly marked as forbidden
  to avoid being misused. (Linus)
- Rebased on top of Linus' repo.
- Link to v1: https://lore.kernel.org/r/20230314-doc-checkpatch-closes-tag-v1-0-1b83072e9a9a@tessares.net

---
Matthieu Baerts (5):
      docs: process: allow Closes tags with links
      checkpatch: don't print the next line if not defined
      checkpatch: use a list of "link" tags
      checkpatch: allow Closes tags with links
      checkpatch: check for misuse of the link tags

 Documentation/process/5.Posting.rst          | 22 ++++++++++----
 Documentation/process/submitting-patches.rst | 26 +++++++++++------
 scripts/checkpatch.pl                        | 43 ++++++++++++++++++++++------
 3 files changed, 70 insertions(+), 21 deletions(-)
---
base-commit: 7e364e56293bb98cae1b55fd835f5991c4e96e7d
change-id: 20230314-doc-checkpatch-closes-tag-1731b57556b1

Best regards,
-- 
Matthieu Baerts <matthieu.baerts@tessares.net>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v4 0/5] docs & checkpatch: allow Closes tags with links
@ 2023-04-03 16:23 ` Matthieu Baerts
  0 siblings, 0 replies; 19+ messages in thread
From: Matthieu Baerts @ 2023-04-03 16:23 UTC (permalink / raw)
  To: Jonathan Corbet, Andy Whitcroft, Joe Perches, Dwaipayan Ray,
	Lukas Bulwahn, Kai Wasserbäch, Thorsten Leemhuis,
	Andrew Morton, David Airlie, Daniel Vetter, Konstantin Ryabitsev,
	Bagas Sanjaya, Linus Torvalds
  Cc: Matthieu Baerts, mptcp, linux-kernel, dri-devel, linux-doc

Since v6.3, checkpatch.pl now complains about the use of "Closes:" tags
followed by a link [1]. It also complains if a "Reported-by:" tag is
followed by a "Closes:" one [2].

As detailed in the first patch, this "Closes:" tag is used for a bit of
time, mainly by DRM and MPTCP subsystems. It is used by some bug
trackers to automate the closure of issues when a patch is accepted.
It is even planned to use this tag with bugzilla.kernel.org [3].

The first patch updates the documentation to explain what is this
"Closes:" tag and how/when to use it. The second patch modifies
checkpatch.pl to stop complaining about it.

The DRM maintainers and their mailing list have been added in Cc as they
are probably interested by these two patches as well.

[1] https://lore.kernel.org/all/3b036087d80b8c0e07a46a1dbaaf4ad0d018f8d5.1674217480.git.linux@leemhuis.info/
[2] https://lore.kernel.org/all/bb5dfd55ea2026303ab2296f4a6df3da7dd64006.1674217480.git.linux@leemhuis.info/
[3] https://lore.kernel.org/linux-doc/20230315181205.f3av7h6owqzzw64p@meerkat.local/

Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
Note: After having re-read the comments from the v1, it is still unclear
to me if this "Closes:" can be accepted or not. But because it seems
that the future Bugzilla bot for kernel.org and regzbot would like to
use it as well, I'm sending here new versions. I'm sorry if I
misunderstood the comments from v1. Please tell me if I did.

Changes in v4:
- Patches 1/5, 3/5 and 4/5 have been added to ask using the "Closes" tag
  instead of the "Link" one for any bug reports. (Thorsten)
- The Fixes tags have been removed from patch 4/5. (Joe)
- The "Reported-by being followed by a link tag" check is now only
  looking for the tag, not the URL which is done elsewhere in patch 5/5.
  (Thorsten)
- A new patch has been added to fix a small issues in checkpatch.pl when
  checking if "Reported-by:" tag is on the last line.
- Link to v3: https://lore.kernel.org/r/20230314-doc-checkpatch-closes-tag-v3-0-d1bdcf31c71c@tessares.net

Changes in v3:
- Patch 1/4 now allow using the "Closes" tag with any kind of bug
  reports, as long as the link is public. (Thorsten)
- The former patch 2/2 has been split in two: first to use a list for
  the different "link" tags (Joe). Then to allow the 'Closes' tag.
- A new patch has been added to let checkpatch.pl checking if "Closes"
  and "Links" are used with a URL.
- Link to v2: https://lore.kernel.org/r/20230314-doc-checkpatch-closes-tag-v2-0-f4a417861f6d@tessares.net

Changes in v2:
- The text on patch 1/2 has been reworked thanks to Jon, Bagas and
  Thorsten. See the individual changelog on the patch for more details.
- Private bug trackers and invalid URLs are clearly marked as forbidden
  to avoid being misused. (Linus)
- Rebased on top of Linus' repo.
- Link to v1: https://lore.kernel.org/r/20230314-doc-checkpatch-closes-tag-v1-0-1b83072e9a9a@tessares.net

---
Matthieu Baerts (5):
      docs: process: allow Closes tags with links
      checkpatch: don't print the next line if not defined
      checkpatch: use a list of "link" tags
      checkpatch: allow Closes tags with links
      checkpatch: check for misuse of the link tags

 Documentation/process/5.Posting.rst          | 22 ++++++++++----
 Documentation/process/submitting-patches.rst | 26 +++++++++++------
 scripts/checkpatch.pl                        | 43 ++++++++++++++++++++++------
 3 files changed, 70 insertions(+), 21 deletions(-)
---
base-commit: 7e364e56293bb98cae1b55fd835f5991c4e96e7d
change-id: 20230314-doc-checkpatch-closes-tag-1731b57556b1

Best regards,
-- 
Matthieu Baerts <matthieu.baerts@tessares.net>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v4 1/5] docs: process: allow Closes tags with links
  2023-04-03 16:23 ` Matthieu Baerts
@ 2023-04-03 16:23   ` Matthieu Baerts
  -1 siblings, 0 replies; 19+ messages in thread
From: Matthieu Baerts @ 2023-04-03 16:23 UTC (permalink / raw)
  To: Jonathan Corbet, Andy Whitcroft, Joe Perches, Dwaipayan Ray,
	Lukas Bulwahn, Kai Wasserbäch, Thorsten Leemhuis,
	Andrew Morton, David Airlie, Daniel Vetter, Konstantin Ryabitsev,
	Bagas Sanjaya, Linus Torvalds
  Cc: linux-doc, linux-kernel, dri-devel, mptcp, Matthieu Baerts

Making sure a bug tracker is up to date is not an easy task. For
example, a first version of a patch fixing a tracked issue can be sent a
long time after having created the issue. But also, it can take some
time to have this patch accepted upstream in its final form. When it is
done, someone -- probably not the person who accepted the patch -- has
to remember about closing the corresponding issue.

This task of closing and tracking the patch can be done automatically by
bug trackers like GitLab [1], GitHub [2] and hopefully soon [3]
bugzilla.kernel.org when the appropriated tag is used. The two first
ones accept multiple tags but it is probably better to pick one.

According to commit 76f381bb77a0 ("checkpatch: warn when unknown tags are used for links"),
the "Closes" tag seems to have been used in the past by a few people and
it is supported by popular bug trackers. Here is how it has been used in
the past:

 $ git log --no-merges --format=email -P --grep='^Closes: http' | \
       grep '^Closes: http' | cut -d/ -f3-5 | sort | uniq -c | sort -rn
    391 gitlab.freedesktop.org/drm/intel
     79 github.com/multipath-tcp/mptcp_net-next
      8 gitlab.freedesktop.org/drm/msm
      3 gitlab.freedesktop.org/drm/amd
      2 gitlab.freedesktop.org/mesa/mesa
      1 patchwork.freedesktop.org/series/73320
      1 gitlab.freedesktop.org/lima/linux
      1 gitlab.freedesktop.org/drm/nouveau
      1 github.com/ClangBuiltLinux/linux
      1 bugzilla.netfilter.org/show_bug.cgi?id=1579
      1 bugzilla.netfilter.org/show_bug.cgi?id=1543
      1 bugzilla.netfilter.org/show_bug.cgi?id=1436
      1 bugzilla.netfilter.org/show_bug.cgi?id=1427
      1 bugs.debian.org/625804

Likely here, the "Closes" tag was only properly used with GitLab and
GitHub. We can also see that it has been used quite a few times (and
still used recently) and this is then not a "random tag that makes no
sense" like it was the case with "BugLink" recently [4]. It has also
been misused but that was a long time ago, when it was common to use
many different random tags.

checkpatch.pl script should then stop complaining about this "Closes"
tag. As suggested by Thorsten [5], if this tag is accepted, it should
first be described in the documentation. This is what is done here in
this patch.

To avoid confusion, the "Closes" should be used with any public bug
report. No need to check if the underlying bug tracker supports
automations. Having this tag with any kind of public bug reports allows
bots like regzbot to clearly identify patches fixing a specific bug and
avoid false-positives, e.g. patches mentioning it is related to an issue
but not fixing it. As suggested by Thorsten [6] again, if we follow the
same logic, the "Closes" tag should then be used after a "Reported-by"
one.

Note that thanks to this "Closes" tag, the mentioned bug trackers can
also locate where a patch has been applied in different branches and
repositories. If only the "Link" tag is used, the tracking can also be
done but the ticket will not be closed and a manual operation will be
needed. Also, these bug trackers have some safeguards: the closure is
only done if a commit having the "Closes:" tag is applied in a specific
branch. It will then not be closed if a random commit having the same
tag is published elsewhere. Also in case of closure, a notification is
sent to the owners.

Link: https://docs.gitlab.com/ee/user/project/issues/managing_issues.html#default-closing-pattern [1]
Link: https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/using-keywords-in-issues-and-pull-requests [2]
Link: https://lore.kernel.org/linux-doc/20230315181205.f3av7h6owqzzw64p@meerkat.local/ [3]
Link: https://lore.kernel.org/all/CAHk-=wgs38ZrfPvy=nOwVkVzjpM3VFU1zobP37Fwd_h9iAD5JQ@mail.gmail.com/ [4]
Link: https://lore.kernel.org/all/688cd6cb-90ab-6834-a6f5-97080e39ca8e@leemhuis.info/ [5]
Link: https://lore.kernel.org/linux-doc/2194d19d-f195-1a1e-41fc-7827ae569351@leemhuis.info/ [6]
Link: https://github.com/multipath-tcp/mptcp_net-next/issues/373
Suggested-by: Thorsten Leemhuis <linux@leemhuis.info>
Acked-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
v4:
- The "Closes" tag should be used for any bug report. Then also always
  after a "Reported-by" tag. (Thorsten Leemhuis)

v3:
- Allow using the "Closes" tag with any bug reports, not only the ones
  supporting automations, useful for regzbot, etc. (Thorsten Leemhuis)

v2:
- Add Konstantin's Acked-by: even if the patch has changed a bit, the
  concept is still the same, I hope that's OK.
- Mention "public" in "5.Posting.rst" file as well. (Jonathan Corbet)
- Re-phrase the new text from "5.Posting.rst". (Bagas Sanjaya &
  Thorsten Leemhuis)
- Clearly mention that private bug trackers and invalid URLs are
  forbidden (Linus Torvalds).
---
 Documentation/process/5.Posting.rst          | 22 +++++++++++++++++-----
 Documentation/process/submitting-patches.rst | 26 ++++++++++++++++++--------
 2 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/Documentation/process/5.Posting.rst b/Documentation/process/5.Posting.rst
index 7a670a075ab6..de4edd42d5c0 100644
--- a/Documentation/process/5.Posting.rst
+++ b/Documentation/process/5.Posting.rst
@@ -207,8 +207,8 @@ the patch::
 	Fixes: 1f2e3d4c5b6a ("The first line of the commit specified by the first 12 characters of its SHA-1 ID")
 
 Another tag is used for linking web pages with additional backgrounds or
-details, for example a report about a bug fixed by the patch or a document
-with a specification implemented by the patch::
+details, for example an earlier discussion which leads to the patch or a
+document with a specification implemented by the patch::
 
 	Link: https://example.com/somewhere.html  optional-other-stuff
 
@@ -217,7 +217,17 @@ latest public review posting of the patch; often this is automatically done
 by tools like b4 or a git hook like the one described in
 'Documentation/maintainer/configure-git.rst'.
 
-A third kind of tag is used to document who was involved in the development of
+If the URL points to a public bug report being fixed by the patch, use the
+"Closes:" tag instead::
+
+	Closes: https://example.com/issues/1234  optional-other-stuff
+
+Some bug trackers have the ability to close issues automatically when a
+commit with such a tag is applied. Some bots monitoring mailing lists can
+also track such tags and take certain actions. Private bug trackers and
+invalid URLs are forbidden.
+
+Another kind of tag is used to document who was involved in the development of
 the patch. Each of these uses this format::
 
 	tag: Full Name <email address>  optional-other-stuff
@@ -251,8 +261,10 @@ The tags in common use are:
  - Reported-by: names a user who reported a problem which is fixed by this
    patch; this tag is used to give credit to the (often underappreciated)
    people who test our code and let us know when things do not work
-   correctly. Note, this tag should be followed by a Link: tag pointing to the
-   report, unless the report is not available on the web.
+   correctly. Note, this tag should be followed by a Closes: tag pointing to
+   the report, unless the report is not available on the web. The Link: tag
+   can be used instead of Closes: if the patch fixes a part of the issue(s)
+   being reported.
 
  - Cc: the named person received a copy of the patch and had the
    opportunity to comment on it.
diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
index 828997bc9ff9..12d58ddc2b8a 100644
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -113,11 +113,9 @@ there is no collision with your six-character ID now, that condition may
 change five years from now.
 
 If related discussions or any other background information behind the change
-can be found on the web, add 'Link:' tags pointing to it. In case your patch
-fixes a bug, for example, add a tag with a URL referencing the report in the
-mailing list archives or a bug tracker; if the patch is a result of some
-earlier mailing list discussion or something documented on the web, point to
-it.
+can be found on the web, add 'Link:' tags pointing to it. If the patch is a
+result of some earlier mailing list discussions or something documented on the
+web, point to it.
 
 When linking to mailing list archives, preferably use the lore.kernel.org
 message archiver service. To create the link URL, use the contents of the
@@ -134,6 +132,16 @@ resources. In addition to giving a URL to a mailing list archive or bug,
 summarize the relevant points of the discussion that led to the
 patch as submitted.
 
+In case your patch fixes a bug, use the 'Closes:' tag with a URL referencing
+the report in the mailing list archives or a public bug tracker. For example::
+
+	Closes: https://example.com/issues/1234
+
+Some bug trackers have the ability to close issues automatically when a
+commit with such a tag is applied. Some bots monitoring mailing lists can
+also track such tags and take certain actions. Private bug trackers and
+invalid URLs are forbidden.
+
 If your patch fixes a bug in a specific commit, e.g. you found an issue using
 ``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
 the SHA-1 ID, and the one line summary.  Do not split the tag across multiple
@@ -498,9 +506,11 @@ Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:
 The Reported-by tag gives credit to people who find bugs and report them and it
 hopefully inspires them to help us again in the future. The tag is intended for
 bugs; please do not use it to credit feature requests. The tag should be
-followed by a Link: tag pointing to the report, unless the report is not
-available on the web. Please note that if the bug was reported in private, then
-ask for permission first before using the Reported-by tag.
+followed by a Closes: tag pointing to the report, unless the report is not
+available on the web. The Link: tag can be used instead of Closes: if the patch
+fixes a part of the issue(s) being reported. Please note that if the bug was
+reported in private, then ask for permission first before using the Reported-by
+tag.
 
 A Tested-by: tag indicates that the patch has been successfully tested (in
 some environment) by the person named.  This tag informs maintainers that

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 1/5] docs: process: allow Closes tags with links
@ 2023-04-03 16:23   ` Matthieu Baerts
  0 siblings, 0 replies; 19+ messages in thread
From: Matthieu Baerts @ 2023-04-03 16:23 UTC (permalink / raw)
  To: Jonathan Corbet, Andy Whitcroft, Joe Perches, Dwaipayan Ray,
	Lukas Bulwahn, Kai Wasserbäch, Thorsten Leemhuis,
	Andrew Morton, David Airlie, Daniel Vetter, Konstantin Ryabitsev,
	Bagas Sanjaya, Linus Torvalds
  Cc: Matthieu Baerts, mptcp, linux-kernel, dri-devel, linux-doc

Making sure a bug tracker is up to date is not an easy task. For
example, a first version of a patch fixing a tracked issue can be sent a
long time after having created the issue. But also, it can take some
time to have this patch accepted upstream in its final form. When it is
done, someone -- probably not the person who accepted the patch -- has
to remember about closing the corresponding issue.

This task of closing and tracking the patch can be done automatically by
bug trackers like GitLab [1], GitHub [2] and hopefully soon [3]
bugzilla.kernel.org when the appropriated tag is used. The two first
ones accept multiple tags but it is probably better to pick one.

According to commit 76f381bb77a0 ("checkpatch: warn when unknown tags are used for links"),
the "Closes" tag seems to have been used in the past by a few people and
it is supported by popular bug trackers. Here is how it has been used in
the past:

 $ git log --no-merges --format=email -P --grep='^Closes: http' | \
       grep '^Closes: http' | cut -d/ -f3-5 | sort | uniq -c | sort -rn
    391 gitlab.freedesktop.org/drm/intel
     79 github.com/multipath-tcp/mptcp_net-next
      8 gitlab.freedesktop.org/drm/msm
      3 gitlab.freedesktop.org/drm/amd
      2 gitlab.freedesktop.org/mesa/mesa
      1 patchwork.freedesktop.org/series/73320
      1 gitlab.freedesktop.org/lima/linux
      1 gitlab.freedesktop.org/drm/nouveau
      1 github.com/ClangBuiltLinux/linux
      1 bugzilla.netfilter.org/show_bug.cgi?id=1579
      1 bugzilla.netfilter.org/show_bug.cgi?id=1543
      1 bugzilla.netfilter.org/show_bug.cgi?id=1436
      1 bugzilla.netfilter.org/show_bug.cgi?id=1427
      1 bugs.debian.org/625804

Likely here, the "Closes" tag was only properly used with GitLab and
GitHub. We can also see that it has been used quite a few times (and
still used recently) and this is then not a "random tag that makes no
sense" like it was the case with "BugLink" recently [4]. It has also
been misused but that was a long time ago, when it was common to use
many different random tags.

checkpatch.pl script should then stop complaining about this "Closes"
tag. As suggested by Thorsten [5], if this tag is accepted, it should
first be described in the documentation. This is what is done here in
this patch.

To avoid confusion, the "Closes" should be used with any public bug
report. No need to check if the underlying bug tracker supports
automations. Having this tag with any kind of public bug reports allows
bots like regzbot to clearly identify patches fixing a specific bug and
avoid false-positives, e.g. patches mentioning it is related to an issue
but not fixing it. As suggested by Thorsten [6] again, if we follow the
same logic, the "Closes" tag should then be used after a "Reported-by"
one.

Note that thanks to this "Closes" tag, the mentioned bug trackers can
also locate where a patch has been applied in different branches and
repositories. If only the "Link" tag is used, the tracking can also be
done but the ticket will not be closed and a manual operation will be
needed. Also, these bug trackers have some safeguards: the closure is
only done if a commit having the "Closes:" tag is applied in a specific
branch. It will then not be closed if a random commit having the same
tag is published elsewhere. Also in case of closure, a notification is
sent to the owners.

Link: https://docs.gitlab.com/ee/user/project/issues/managing_issues.html#default-closing-pattern [1]
Link: https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/using-keywords-in-issues-and-pull-requests [2]
Link: https://lore.kernel.org/linux-doc/20230315181205.f3av7h6owqzzw64p@meerkat.local/ [3]
Link: https://lore.kernel.org/all/CAHk-=wgs38ZrfPvy=nOwVkVzjpM3VFU1zobP37Fwd_h9iAD5JQ@mail.gmail.com/ [4]
Link: https://lore.kernel.org/all/688cd6cb-90ab-6834-a6f5-97080e39ca8e@leemhuis.info/ [5]
Link: https://lore.kernel.org/linux-doc/2194d19d-f195-1a1e-41fc-7827ae569351@leemhuis.info/ [6]
Link: https://github.com/multipath-tcp/mptcp_net-next/issues/373
Suggested-by: Thorsten Leemhuis <linux@leemhuis.info>
Acked-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
v4:
- The "Closes" tag should be used for any bug report. Then also always
  after a "Reported-by" tag. (Thorsten Leemhuis)

v3:
- Allow using the "Closes" tag with any bug reports, not only the ones
  supporting automations, useful for regzbot, etc. (Thorsten Leemhuis)

v2:
- Add Konstantin's Acked-by: even if the patch has changed a bit, the
  concept is still the same, I hope that's OK.
- Mention "public" in "5.Posting.rst" file as well. (Jonathan Corbet)
- Re-phrase the new text from "5.Posting.rst". (Bagas Sanjaya &
  Thorsten Leemhuis)
- Clearly mention that private bug trackers and invalid URLs are
  forbidden (Linus Torvalds).
---
 Documentation/process/5.Posting.rst          | 22 +++++++++++++++++-----
 Documentation/process/submitting-patches.rst | 26 ++++++++++++++++++--------
 2 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/Documentation/process/5.Posting.rst b/Documentation/process/5.Posting.rst
index 7a670a075ab6..de4edd42d5c0 100644
--- a/Documentation/process/5.Posting.rst
+++ b/Documentation/process/5.Posting.rst
@@ -207,8 +207,8 @@ the patch::
 	Fixes: 1f2e3d4c5b6a ("The first line of the commit specified by the first 12 characters of its SHA-1 ID")
 
 Another tag is used for linking web pages with additional backgrounds or
-details, for example a report about a bug fixed by the patch or a document
-with a specification implemented by the patch::
+details, for example an earlier discussion which leads to the patch or a
+document with a specification implemented by the patch::
 
 	Link: https://example.com/somewhere.html  optional-other-stuff
 
@@ -217,7 +217,17 @@ latest public review posting of the patch; often this is automatically done
 by tools like b4 or a git hook like the one described in
 'Documentation/maintainer/configure-git.rst'.
 
-A third kind of tag is used to document who was involved in the development of
+If the URL points to a public bug report being fixed by the patch, use the
+"Closes:" tag instead::
+
+	Closes: https://example.com/issues/1234  optional-other-stuff
+
+Some bug trackers have the ability to close issues automatically when a
+commit with such a tag is applied. Some bots monitoring mailing lists can
+also track such tags and take certain actions. Private bug trackers and
+invalid URLs are forbidden.
+
+Another kind of tag is used to document who was involved in the development of
 the patch. Each of these uses this format::
 
 	tag: Full Name <email address>  optional-other-stuff
@@ -251,8 +261,10 @@ The tags in common use are:
  - Reported-by: names a user who reported a problem which is fixed by this
    patch; this tag is used to give credit to the (often underappreciated)
    people who test our code and let us know when things do not work
-   correctly. Note, this tag should be followed by a Link: tag pointing to the
-   report, unless the report is not available on the web.
+   correctly. Note, this tag should be followed by a Closes: tag pointing to
+   the report, unless the report is not available on the web. The Link: tag
+   can be used instead of Closes: if the patch fixes a part of the issue(s)
+   being reported.
 
  - Cc: the named person received a copy of the patch and had the
    opportunity to comment on it.
diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
index 828997bc9ff9..12d58ddc2b8a 100644
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -113,11 +113,9 @@ there is no collision with your six-character ID now, that condition may
 change five years from now.
 
 If related discussions or any other background information behind the change
-can be found on the web, add 'Link:' tags pointing to it. In case your patch
-fixes a bug, for example, add a tag with a URL referencing the report in the
-mailing list archives or a bug tracker; if the patch is a result of some
-earlier mailing list discussion or something documented on the web, point to
-it.
+can be found on the web, add 'Link:' tags pointing to it. If the patch is a
+result of some earlier mailing list discussions or something documented on the
+web, point to it.
 
 When linking to mailing list archives, preferably use the lore.kernel.org
 message archiver service. To create the link URL, use the contents of the
@@ -134,6 +132,16 @@ resources. In addition to giving a URL to a mailing list archive or bug,
 summarize the relevant points of the discussion that led to the
 patch as submitted.
 
+In case your patch fixes a bug, use the 'Closes:' tag with a URL referencing
+the report in the mailing list archives or a public bug tracker. For example::
+
+	Closes: https://example.com/issues/1234
+
+Some bug trackers have the ability to close issues automatically when a
+commit with such a tag is applied. Some bots monitoring mailing lists can
+also track such tags and take certain actions. Private bug trackers and
+invalid URLs are forbidden.
+
 If your patch fixes a bug in a specific commit, e.g. you found an issue using
 ``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
 the SHA-1 ID, and the one line summary.  Do not split the tag across multiple
@@ -498,9 +506,11 @@ Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:
 The Reported-by tag gives credit to people who find bugs and report them and it
 hopefully inspires them to help us again in the future. The tag is intended for
 bugs; please do not use it to credit feature requests. The tag should be
-followed by a Link: tag pointing to the report, unless the report is not
-available on the web. Please note that if the bug was reported in private, then
-ask for permission first before using the Reported-by tag.
+followed by a Closes: tag pointing to the report, unless the report is not
+available on the web. The Link: tag can be used instead of Closes: if the patch
+fixes a part of the issue(s) being reported. Please note that if the bug was
+reported in private, then ask for permission first before using the Reported-by
+tag.
 
 A Tested-by: tag indicates that the patch has been successfully tested (in
 some environment) by the person named.  This tag informs maintainers that

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 2/5] checkpatch: don't print the next line if not defined
  2023-04-03 16:23 ` Matthieu Baerts
@ 2023-04-03 16:23   ` Matthieu Baerts
  -1 siblings, 0 replies; 19+ messages in thread
From: Matthieu Baerts @ 2023-04-03 16:23 UTC (permalink / raw)
  To: Jonathan Corbet, Andy Whitcroft, Joe Perches, Dwaipayan Ray,
	Lukas Bulwahn, Kai Wasserbäch, Thorsten Leemhuis,
	Andrew Morton, David Airlie, Daniel Vetter, Konstantin Ryabitsev,
	Bagas Sanjaya, Linus Torvalds
  Cc: linux-doc, linux-kernel, dri-devel, mptcp, Matthieu Baerts

When checking if "Reported-by" tag is followed by "Link:", there is no
need to print the next line if there is no next line.

While at it, also mention in this case that the "Link:" tag should be
followed by a URL, similar to the next warning.

By doing that, the code is now similar to what is done above when
checking if the Co-developed-by tag is properly used.

Fixes: d7f1d71e5ef6 ("checkpatch: warn when Reported-by: is not followed by Link:")
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
v4:
 - Add new patch.
---
 scripts/checkpatch.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index bd44d12965c9..b170fc7ef258 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3162,7 +3162,7 @@ sub process {
 			if ($sign_off =~ /^reported(?:|-and-tested)-by:$/i) {
 				if (!defined $lines[$linenr]) {
 					WARN("BAD_REPORTED_BY_LINK",
-					     "Reported-by: should be immediately followed by Link: to the report\n" . $herecurr . $rawlines[$linenr] . "\n");
+					     "Reported-by: should be immediately followed by Link: with a URL to the report\n" . $herecurr . "\n");
 				} elsif ($rawlines[$linenr] !~ m{^link:\s*https?://}i) {
 					WARN("BAD_REPORTED_BY_LINK",
 					     "Reported-by: should be immediately followed by Link: with a URL to the report\n" . $herecurr . $rawlines[$linenr] . "\n");

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 2/5] checkpatch: don't print the next line if not defined
@ 2023-04-03 16:23   ` Matthieu Baerts
  0 siblings, 0 replies; 19+ messages in thread
From: Matthieu Baerts @ 2023-04-03 16:23 UTC (permalink / raw)
  To: Jonathan Corbet, Andy Whitcroft, Joe Perches, Dwaipayan Ray,
	Lukas Bulwahn, Kai Wasserbäch, Thorsten Leemhuis,
	Andrew Morton, David Airlie, Daniel Vetter, Konstantin Ryabitsev,
	Bagas Sanjaya, Linus Torvalds
  Cc: Matthieu Baerts, mptcp, linux-kernel, dri-devel, linux-doc

When checking if "Reported-by" tag is followed by "Link:", there is no
need to print the next line if there is no next line.

While at it, also mention in this case that the "Link:" tag should be
followed by a URL, similar to the next warning.

By doing that, the code is now similar to what is done above when
checking if the Co-developed-by tag is properly used.

Fixes: d7f1d71e5ef6 ("checkpatch: warn when Reported-by: is not followed by Link:")
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
v4:
 - Add new patch.
---
 scripts/checkpatch.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index bd44d12965c9..b170fc7ef258 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3162,7 +3162,7 @@ sub process {
 			if ($sign_off =~ /^reported(?:|-and-tested)-by:$/i) {
 				if (!defined $lines[$linenr]) {
 					WARN("BAD_REPORTED_BY_LINK",
-					     "Reported-by: should be immediately followed by Link: to the report\n" . $herecurr . $rawlines[$linenr] . "\n");
+					     "Reported-by: should be immediately followed by Link: with a URL to the report\n" . $herecurr . "\n");
 				} elsif ($rawlines[$linenr] !~ m{^link:\s*https?://}i) {
 					WARN("BAD_REPORTED_BY_LINK",
 					     "Reported-by: should be immediately followed by Link: with a URL to the report\n" . $herecurr . $rawlines[$linenr] . "\n");

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 3/5] checkpatch: use a list of "link" tags
  2023-04-03 16:23 ` Matthieu Baerts
@ 2023-04-03 16:23   ` Matthieu Baerts
  -1 siblings, 0 replies; 19+ messages in thread
From: Matthieu Baerts @ 2023-04-03 16:23 UTC (permalink / raw)
  To: Jonathan Corbet, Andy Whitcroft, Joe Perches, Dwaipayan Ray,
	Lukas Bulwahn, Kai Wasserbäch, Thorsten Leemhuis,
	Andrew Morton, David Airlie, Daniel Vetter, Konstantin Ryabitsev,
	Bagas Sanjaya, Linus Torvalds
  Cc: linux-doc, linux-kernel, dri-devel, mptcp, Matthieu Baerts

The following commit will allow the use of a similar "link" tag.

Because there is a possibility that other similar tags will be added in
the future and to reduce the number of places where the code will be
modified to allow this new tag, a list with all these "link" tags is now
used.

Two variables are created from it: one to search for such tags and one
to print all tags in a warning message.

Suggested-by: Joe Perches <joe@perches.com>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
v4:
 - "Reported-by:" should be followed by a "Closes:" tag. (Thorsten
   Leemhuis)

v3:
 - new patch. (Joe Perches)
---
 scripts/checkpatch.pl | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b170fc7ef258..1647ef72480e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -620,6 +620,22 @@ our $signature_tags = qr{(?xi:
 	Cc:
 )};
 
+our @link_tags = qw(Link);
+
+#Create a search and print patterns for all these strings to be used directly below
+our $link_tags_search = "";
+our $link_tags_print = "";
+foreach my $entry (@link_tags) {
+	if ($link_tags_search ne "") {
+		$link_tags_search .= '|';
+		$link_tags_print .= ' or ';
+	}
+	$entry .= ':';
+	$link_tags_search .= $entry;
+	$link_tags_print .= "'$entry'";
+}
+$link_tags_search = "(?:${link_tags_search})";
+
 our $tracing_logging_tags = qr{(?xi:
 	[=-]*> |
 	<[=-]* |
@@ -3250,8 +3266,8 @@ sub process {
 					# file delta changes
 		      $line =~ /^\s*(?:[\w\.\-\+]*\/)++[\w\.\-\+]+:/ ||
 					# filename then :
-		      $line =~ /^\s*(?:Fixes:|Link:|$signature_tags)/i ||
-					# A Fixes: or Link: line or signature tag line
+		      $line =~ /^\s*(?:Fixes:|$link_tags_search|$signature_tags)/i ||
+					# A Fixes:, link or signature tag line
 		      $commit_log_possible_stack_dump)) {
 			WARN("COMMIT_LOG_LONG_LINE",
 			     "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
@@ -3266,13 +3282,13 @@ sub process {
 
 # Check for odd tags before a URI/URL
 		if ($in_commit_log &&
-		    $line =~ /^\s*(\w+):\s*http/ && $1 ne 'Link') {
+		    $line =~ /^\s*(\w+:)\s*http/ && $1 !~ /^$link_tags_search$/) {
 			if ($1 =~ /^v(?:ersion)?\d+/i) {
 				WARN("COMMIT_LOG_VERSIONING",
 				     "Patch version information should be after the --- line\n" . $herecurr);
 			} else {
 				WARN("COMMIT_LOG_USE_LINK",
-				     "Unknown link reference '$1:', use 'Link:' instead\n" . $herecurr);
+				     "Unknown link reference '$1', use $link_tags_print instead\n" . $herecurr);
 			}
 		}
 

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 3/5] checkpatch: use a list of "link" tags
@ 2023-04-03 16:23   ` Matthieu Baerts
  0 siblings, 0 replies; 19+ messages in thread
From: Matthieu Baerts @ 2023-04-03 16:23 UTC (permalink / raw)
  To: Jonathan Corbet, Andy Whitcroft, Joe Perches, Dwaipayan Ray,
	Lukas Bulwahn, Kai Wasserbäch, Thorsten Leemhuis,
	Andrew Morton, David Airlie, Daniel Vetter, Konstantin Ryabitsev,
	Bagas Sanjaya, Linus Torvalds
  Cc: Matthieu Baerts, mptcp, linux-kernel, dri-devel, linux-doc

The following commit will allow the use of a similar "link" tag.

Because there is a possibility that other similar tags will be added in
the future and to reduce the number of places where the code will be
modified to allow this new tag, a list with all these "link" tags is now
used.

Two variables are created from it: one to search for such tags and one
to print all tags in a warning message.

Suggested-by: Joe Perches <joe@perches.com>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
v4:
 - "Reported-by:" should be followed by a "Closes:" tag. (Thorsten
   Leemhuis)

v3:
 - new patch. (Joe Perches)
---
 scripts/checkpatch.pl | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b170fc7ef258..1647ef72480e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -620,6 +620,22 @@ our $signature_tags = qr{(?xi:
 	Cc:
 )};
 
+our @link_tags = qw(Link);
+
+#Create a search and print patterns for all these strings to be used directly below
+our $link_tags_search = "";
+our $link_tags_print = "";
+foreach my $entry (@link_tags) {
+	if ($link_tags_search ne "") {
+		$link_tags_search .= '|';
+		$link_tags_print .= ' or ';
+	}
+	$entry .= ':';
+	$link_tags_search .= $entry;
+	$link_tags_print .= "'$entry'";
+}
+$link_tags_search = "(?:${link_tags_search})";
+
 our $tracing_logging_tags = qr{(?xi:
 	[=-]*> |
 	<[=-]* |
@@ -3250,8 +3266,8 @@ sub process {
 					# file delta changes
 		      $line =~ /^\s*(?:[\w\.\-\+]*\/)++[\w\.\-\+]+:/ ||
 					# filename then :
-		      $line =~ /^\s*(?:Fixes:|Link:|$signature_tags)/i ||
-					# A Fixes: or Link: line or signature tag line
+		      $line =~ /^\s*(?:Fixes:|$link_tags_search|$signature_tags)/i ||
+					# A Fixes:, link or signature tag line
 		      $commit_log_possible_stack_dump)) {
 			WARN("COMMIT_LOG_LONG_LINE",
 			     "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
@@ -3266,13 +3282,13 @@ sub process {
 
 # Check for odd tags before a URI/URL
 		if ($in_commit_log &&
-		    $line =~ /^\s*(\w+):\s*http/ && $1 ne 'Link') {
+		    $line =~ /^\s*(\w+:)\s*http/ && $1 !~ /^$link_tags_search$/) {
 			if ($1 =~ /^v(?:ersion)?\d+/i) {
 				WARN("COMMIT_LOG_VERSIONING",
 				     "Patch version information should be after the --- line\n" . $herecurr);
 			} else {
 				WARN("COMMIT_LOG_USE_LINK",
-				     "Unknown link reference '$1:', use 'Link:' instead\n" . $herecurr);
+				     "Unknown link reference '$1', use $link_tags_print instead\n" . $herecurr);
 			}
 		}
 

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 4/5] checkpatch: allow Closes tags with links
  2023-04-03 16:23 ` Matthieu Baerts
@ 2023-04-03 16:23   ` Matthieu Baerts
  -1 siblings, 0 replies; 19+ messages in thread
From: Matthieu Baerts @ 2023-04-03 16:23 UTC (permalink / raw)
  To: Jonathan Corbet, Andy Whitcroft, Joe Perches, Dwaipayan Ray,
	Lukas Bulwahn, Kai Wasserbäch, Thorsten Leemhuis,
	Andrew Morton, David Airlie, Daniel Vetter, Konstantin Ryabitsev,
	Bagas Sanjaya, Linus Torvalds
  Cc: linux-doc, linux-kernel, dri-devel, mptcp, Matthieu Baerts

As a follow-up of a previous patch modifying the documentation to
allow using the "Closes:" tag, checkpatch.pl is updated accordingly.

checkpatch.pl now no longer complain when the "Closes:" tag is used by
itself:

  commit 76f381bb77a0 ("checkpatch: warn when unknown tags are used for links")

... or after the "Reported-by:" tag:

  commit d7f1d71e5ef6 ("checkpatch: warn when Reported-by: is not followed by Link:")

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/373
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
v4:
 - remove the "Fixes" tags. (Joe Perches)
 - "Reported-by:" should be followed by a "Closes:" tag. (Thorsten
   Leemhuis)

v3:
 - split into 2 patches: the previous one adds a list with all the
   "link" tags. This one only allows the "Closes" tag. (Joe Perches)
 - "Closes" is no longer printed between parenthesis. (Thorsten
   Leemhuis)
---
 scripts/checkpatch.pl | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1647ef72480e..f8a57f400570 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -620,7 +620,7 @@ our $signature_tags = qr{(?xi:
 	Cc:
 )};
 
-our @link_tags = qw(Link);
+our @link_tags = qw(Link Closes);
 
 #Create a search and print patterns for all these strings to be used directly below
 our $link_tags_search = "";
@@ -3174,14 +3174,14 @@ sub process {
 				}
 			}
 
-# check if Reported-by: is followed by a Link:
+# check if Reported-by: is followed by a Closes: tag
 			if ($sign_off =~ /^reported(?:|-and-tested)-by:$/i) {
 				if (!defined $lines[$linenr]) {
 					WARN("BAD_REPORTED_BY_LINK",
-					     "Reported-by: should be immediately followed by Link: with a URL to the report\n" . $herecurr . "\n");
-				} elsif ($rawlines[$linenr] !~ m{^link:\s*https?://}i) {
+					     "Reported-by: should be immediately followed by Closes: with a URL to the report\n" . $herecurr . "\n");
+				} elsif ($rawlines[$linenr] !~ m{^closes:\s*https?://}i) {
 					WARN("BAD_REPORTED_BY_LINK",
-					     "Reported-by: should be immediately followed by Link: with a URL to the report\n" . $herecurr . $rawlines[$linenr] . "\n");
+					     "Reported-by: should be immediately followed by Closes: with a URL to the report\n" . $herecurr . $rawlines[$linenr] . "\n");
 				}
 			}
 		}

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 4/5] checkpatch: allow Closes tags with links
@ 2023-04-03 16:23   ` Matthieu Baerts
  0 siblings, 0 replies; 19+ messages in thread
From: Matthieu Baerts @ 2023-04-03 16:23 UTC (permalink / raw)
  To: Jonathan Corbet, Andy Whitcroft, Joe Perches, Dwaipayan Ray,
	Lukas Bulwahn, Kai Wasserbäch, Thorsten Leemhuis,
	Andrew Morton, David Airlie, Daniel Vetter, Konstantin Ryabitsev,
	Bagas Sanjaya, Linus Torvalds
  Cc: Matthieu Baerts, mptcp, linux-kernel, dri-devel, linux-doc

As a follow-up of a previous patch modifying the documentation to
allow using the "Closes:" tag, checkpatch.pl is updated accordingly.

checkpatch.pl now no longer complain when the "Closes:" tag is used by
itself:

  commit 76f381bb77a0 ("checkpatch: warn when unknown tags are used for links")

... or after the "Reported-by:" tag:

  commit d7f1d71e5ef6 ("checkpatch: warn when Reported-by: is not followed by Link:")

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/373
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
v4:
 - remove the "Fixes" tags. (Joe Perches)
 - "Reported-by:" should be followed by a "Closes:" tag. (Thorsten
   Leemhuis)

v3:
 - split into 2 patches: the previous one adds a list with all the
   "link" tags. This one only allows the "Closes" tag. (Joe Perches)
 - "Closes" is no longer printed between parenthesis. (Thorsten
   Leemhuis)
---
 scripts/checkpatch.pl | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1647ef72480e..f8a57f400570 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -620,7 +620,7 @@ our $signature_tags = qr{(?xi:
 	Cc:
 )};
 
-our @link_tags = qw(Link);
+our @link_tags = qw(Link Closes);
 
 #Create a search and print patterns for all these strings to be used directly below
 our $link_tags_search = "";
@@ -3174,14 +3174,14 @@ sub process {
 				}
 			}
 
-# check if Reported-by: is followed by a Link:
+# check if Reported-by: is followed by a Closes: tag
 			if ($sign_off =~ /^reported(?:|-and-tested)-by:$/i) {
 				if (!defined $lines[$linenr]) {
 					WARN("BAD_REPORTED_BY_LINK",
-					     "Reported-by: should be immediately followed by Link: with a URL to the report\n" . $herecurr . "\n");
-				} elsif ($rawlines[$linenr] !~ m{^link:\s*https?://}i) {
+					     "Reported-by: should be immediately followed by Closes: with a URL to the report\n" . $herecurr . "\n");
+				} elsif ($rawlines[$linenr] !~ m{^closes:\s*https?://}i) {
 					WARN("BAD_REPORTED_BY_LINK",
-					     "Reported-by: should be immediately followed by Link: with a URL to the report\n" . $herecurr . $rawlines[$linenr] . "\n");
+					     "Reported-by: should be immediately followed by Closes: with a URL to the report\n" . $herecurr . $rawlines[$linenr] . "\n");
 				}
 			}
 		}

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 5/5] checkpatch: check for misuse of the link tags
  2023-04-03 16:23 ` Matthieu Baerts
@ 2023-04-03 16:23   ` Matthieu Baerts
  -1 siblings, 0 replies; 19+ messages in thread
From: Matthieu Baerts @ 2023-04-03 16:23 UTC (permalink / raw)
  To: Jonathan Corbet, Andy Whitcroft, Joe Perches, Dwaipayan Ray,
	Lukas Bulwahn, Kai Wasserbäch, Thorsten Leemhuis,
	Andrew Morton, David Airlie, Daniel Vetter, Konstantin Ryabitsev,
	Bagas Sanjaya, Linus Torvalds
  Cc: linux-doc, linux-kernel, dri-devel, mptcp, Matthieu Baerts

"Link:" and "Closes:" tags have to be used with public URLs.

It is difficult to make sure the link is public but at least we can
verify the tag is followed by 'http(s)://'.

With that, we avoid such a tag that is not allowed [1]:

  Closes: <number>

Now that we check the "link" tags are followed by a URL, we can relax
the check linked to "Reported-by being followed by a link tag" to only
verify if a "link" tag is present after the "Reported-by" one.

Link: https://lore.kernel.org/linux-doc/CAHk-=wh0v1EeDV3v8TzK81nDC40=XuTdY2MCr0xy3m3FiBV3+Q@mail.gmail.com/ [1]
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
v4:
- Relax "Reported-by being followed by a link tag" check
- Check for "http(s)://" not just "http(s):"

v3:
- new patch addressing Linus' concerns.
---
 scripts/checkpatch.pl | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index f8a57f400570..6d602c61d72a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3179,7 +3179,7 @@ sub process {
 				if (!defined $lines[$linenr]) {
 					WARN("BAD_REPORTED_BY_LINK",
 					     "Reported-by: should be immediately followed by Closes: with a URL to the report\n" . $herecurr . "\n");
-				} elsif ($rawlines[$linenr] !~ m{^closes:\s*https?://}i) {
+				} elsif ($rawlines[$linenr] !~ /^closes:\s*/i) {
 					WARN("BAD_REPORTED_BY_LINK",
 					     "Reported-by: should be immediately followed by Closes: with a URL to the report\n" . $herecurr . $rawlines[$linenr] . "\n");
 				}
@@ -3292,6 +3292,17 @@ sub process {
 			}
 		}
 
+# Check for misuse of the link tags
+		if ($in_commit_log &&
+		    $line =~ /^\s*(\w+:)\s*(\S+)/) {
+			my $tag = $1;
+			my $value = $2;
+			if ($tag =~ /^$link_tags_search$/ && $value !~ m{^https?://}) {
+				WARN("COMMIT_LOG_WRONG_LINK",
+				     "'$tag' should be followed by a public http(s) link\n" . $herecurr);
+			}
+		}
+
 # Check for lines starting with a #
 		if ($in_commit_log && $line =~ /^#/) {
 			if (WARN("COMMIT_COMMENT_SYMBOL",

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 5/5] checkpatch: check for misuse of the link tags
@ 2023-04-03 16:23   ` Matthieu Baerts
  0 siblings, 0 replies; 19+ messages in thread
From: Matthieu Baerts @ 2023-04-03 16:23 UTC (permalink / raw)
  To: Jonathan Corbet, Andy Whitcroft, Joe Perches, Dwaipayan Ray,
	Lukas Bulwahn, Kai Wasserbäch, Thorsten Leemhuis,
	Andrew Morton, David Airlie, Daniel Vetter, Konstantin Ryabitsev,
	Bagas Sanjaya, Linus Torvalds
  Cc: Matthieu Baerts, mptcp, linux-kernel, dri-devel, linux-doc

"Link:" and "Closes:" tags have to be used with public URLs.

It is difficult to make sure the link is public but at least we can
verify the tag is followed by 'http(s)://'.

With that, we avoid such a tag that is not allowed [1]:

  Closes: <number>

Now that we check the "link" tags are followed by a URL, we can relax
the check linked to "Reported-by being followed by a link tag" to only
verify if a "link" tag is present after the "Reported-by" one.

Link: https://lore.kernel.org/linux-doc/CAHk-=wh0v1EeDV3v8TzK81nDC40=XuTdY2MCr0xy3m3FiBV3+Q@mail.gmail.com/ [1]
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
v4:
- Relax "Reported-by being followed by a link tag" check
- Check for "http(s)://" not just "http(s):"

v3:
- new patch addressing Linus' concerns.
---
 scripts/checkpatch.pl | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index f8a57f400570..6d602c61d72a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3179,7 +3179,7 @@ sub process {
 				if (!defined $lines[$linenr]) {
 					WARN("BAD_REPORTED_BY_LINK",
 					     "Reported-by: should be immediately followed by Closes: with a URL to the report\n" . $herecurr . "\n");
-				} elsif ($rawlines[$linenr] !~ m{^closes:\s*https?://}i) {
+				} elsif ($rawlines[$linenr] !~ /^closes:\s*/i) {
 					WARN("BAD_REPORTED_BY_LINK",
 					     "Reported-by: should be immediately followed by Closes: with a URL to the report\n" . $herecurr . $rawlines[$linenr] . "\n");
 				}
@@ -3292,6 +3292,17 @@ sub process {
 			}
 		}
 
+# Check for misuse of the link tags
+		if ($in_commit_log &&
+		    $line =~ /^\s*(\w+:)\s*(\S+)/) {
+			my $tag = $1;
+			my $value = $2;
+			if ($tag =~ /^$link_tags_search$/ && $value !~ m{^https?://}) {
+				WARN("COMMIT_LOG_WRONG_LINK",
+				     "'$tag' should be followed by a public http(s) link\n" . $herecurr);
+			}
+		}
+
 # Check for lines starting with a #
 		if ($in_commit_log && $line =~ /^#/) {
 			if (WARN("COMMIT_COMMENT_SYMBOL",

-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v4 0/5] docs & checkpatch: allow Closes tags with links
  2023-04-03 16:23 ` Matthieu Baerts
@ 2023-04-03 16:44   ` Joe Perches
  -1 siblings, 0 replies; 19+ messages in thread
From: Joe Perches @ 2023-04-03 16:44 UTC (permalink / raw)
  To: Matthieu Baerts, Jonathan Corbet, Andy Whitcroft, Dwaipayan Ray,
	Lukas Bulwahn, Kai Wasserbäch, Thorsten Leemhuis,
	Andrew Morton, David Airlie, Daniel Vetter, Konstantin Ryabitsev,
	Bagas Sanjaya, Linus Torvalds
  Cc: linux-doc, linux-kernel, dri-devel, mptcp

On Mon, 2023-04-03 at 18:23 +0200, Matthieu Baerts wrote:
> Since v6.3, checkpatch.pl now complains about the use of "Closes:" tags
> followed by a link [1]. It also complains if a "Reported-by:" tag is
> followed by a "Closes:" one [2].

All these patches seems sensible, thanks.

Assuming Linus approves the use of "Closes:"

Acked-by: Joe Perches <joe@perches.com>

> As detailed in the first patch, this "Closes:" tag is used for a bit of
> time, mainly by DRM and MPTCP subsystems. It is used by some bug
> trackers to automate the closure of issues when a patch is accepted.
> It is even planned to use this tag with bugzilla.kernel.org [3].
> 
> The first patch updates the documentation to explain what is this
> "Closes:" tag and how/when to use it. The second patch modifies
> checkpatch.pl to stop complaining about it.
> 
> The DRM maintainers and their mailing list have been added in Cc as they
> are probably interested by these two patches as well.
> 
> [1] https://lore.kernel.org/all/3b036087d80b8c0e07a46a1dbaaf4ad0d018f8d5.1674217480.git.linux@leemhuis.info/
> [2] https://lore.kernel.org/all/bb5dfd55ea2026303ab2296f4a6df3da7dd64006.1674217480.git.linux@leemhuis.info/
> [3] https://lore.kernel.org/linux-doc/20230315181205.f3av7h6owqzzw64p@meerkat.local/
> 
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> ---
> Note: After having re-read the comments from the v1, it is still unclear
> to me if this "Closes:" can be accepted or not. But because it seems
> that the future Bugzilla bot for kernel.org and regzbot would like to
> use it as well, I'm sending here new versions. I'm sorry if I
> misunderstood the comments from v1. Please tell me if I did.
> 
> Changes in v4:
> - Patches 1/5, 3/5 and 4/5 have been added to ask using the "Closes" tag
>   instead of the "Link" one for any bug reports. (Thorsten)
> - The Fixes tags have been removed from patch 4/5. (Joe)
> - The "Reported-by being followed by a link tag" check is now only
>   looking for the tag, not the URL which is done elsewhere in patch 5/5.
>   (Thorsten)
> - A new patch has been added to fix a small issues in checkpatch.pl when
>   checking if "Reported-by:" tag is on the last line.
> - Link to v3: https://lore.kernel.org/r/20230314-doc-checkpatch-closes-tag-v3-0-d1bdcf31c71c@tessares.net
> 
> Changes in v3:
> - Patch 1/4 now allow using the "Closes" tag with any kind of bug
>   reports, as long as the link is public. (Thorsten)
> - The former patch 2/2 has been split in two: first to use a list for
>   the different "link" tags (Joe). Then to allow the 'Closes' tag.
> - A new patch has been added to let checkpatch.pl checking if "Closes"
>   and "Links" are used with a URL.
> - Link to v2: https://lore.kernel.org/r/20230314-doc-checkpatch-closes-tag-v2-0-f4a417861f6d@tessares.net
> 
> Changes in v2:
> - The text on patch 1/2 has been reworked thanks to Jon, Bagas and
>   Thorsten. See the individual changelog on the patch for more details.
> - Private bug trackers and invalid URLs are clearly marked as forbidden
>   to avoid being misused. (Linus)
> - Rebased on top of Linus' repo.
> - Link to v1: https://lore.kernel.org/r/20230314-doc-checkpatch-closes-tag-v1-0-1b83072e9a9a@tessares.net
> 
> ---
> Matthieu Baerts (5):
>       docs: process: allow Closes tags with links
>       checkpatch: don't print the next line if not defined
>       checkpatch: use a list of "link" tags
>       checkpatch: allow Closes tags with links
>       checkpatch: check for misuse of the link tags
> 
>  Documentation/process/5.Posting.rst          | 22 ++++++++++----
>  Documentation/process/submitting-patches.rst | 26 +++++++++++------
>  scripts/checkpatch.pl                        | 43 ++++++++++++++++++++++------
>  3 files changed, 70 insertions(+), 21 deletions(-)
> ---
> base-commit: 7e364e56293bb98cae1b55fd835f5991c4e96e7d
> change-id: 20230314-doc-checkpatch-closes-tag-1731b57556b1
> 
> Best regards,


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v4 0/5] docs & checkpatch: allow Closes tags with links
@ 2023-04-03 16:44   ` Joe Perches
  0 siblings, 0 replies; 19+ messages in thread
From: Joe Perches @ 2023-04-03 16:44 UTC (permalink / raw)
  To: Matthieu Baerts, Jonathan Corbet, Andy Whitcroft, Dwaipayan Ray,
	Lukas Bulwahn, Kai Wasserbäch, Thorsten Leemhuis,
	Andrew Morton, David Airlie, Daniel Vetter, Konstantin Ryabitsev,
	Bagas Sanjaya, Linus Torvalds
  Cc: mptcp, linux-kernel, dri-devel, linux-doc

On Mon, 2023-04-03 at 18:23 +0200, Matthieu Baerts wrote:
> Since v6.3, checkpatch.pl now complains about the use of "Closes:" tags
> followed by a link [1]. It also complains if a "Reported-by:" tag is
> followed by a "Closes:" one [2].

All these patches seems sensible, thanks.

Assuming Linus approves the use of "Closes:"

Acked-by: Joe Perches <joe@perches.com>

> As detailed in the first patch, this "Closes:" tag is used for a bit of
> time, mainly by DRM and MPTCP subsystems. It is used by some bug
> trackers to automate the closure of issues when a patch is accepted.
> It is even planned to use this tag with bugzilla.kernel.org [3].
> 
> The first patch updates the documentation to explain what is this
> "Closes:" tag and how/when to use it. The second patch modifies
> checkpatch.pl to stop complaining about it.
> 
> The DRM maintainers and their mailing list have been added in Cc as they
> are probably interested by these two patches as well.
> 
> [1] https://lore.kernel.org/all/3b036087d80b8c0e07a46a1dbaaf4ad0d018f8d5.1674217480.git.linux@leemhuis.info/
> [2] https://lore.kernel.org/all/bb5dfd55ea2026303ab2296f4a6df3da7dd64006.1674217480.git.linux@leemhuis.info/
> [3] https://lore.kernel.org/linux-doc/20230315181205.f3av7h6owqzzw64p@meerkat.local/
> 
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> ---
> Note: After having re-read the comments from the v1, it is still unclear
> to me if this "Closes:" can be accepted or not. But because it seems
> that the future Bugzilla bot for kernel.org and regzbot would like to
> use it as well, I'm sending here new versions. I'm sorry if I
> misunderstood the comments from v1. Please tell me if I did.
> 
> Changes in v4:
> - Patches 1/5, 3/5 and 4/5 have been added to ask using the "Closes" tag
>   instead of the "Link" one for any bug reports. (Thorsten)
> - The Fixes tags have been removed from patch 4/5. (Joe)
> - The "Reported-by being followed by a link tag" check is now only
>   looking for the tag, not the URL which is done elsewhere in patch 5/5.
>   (Thorsten)
> - A new patch has been added to fix a small issues in checkpatch.pl when
>   checking if "Reported-by:" tag is on the last line.
> - Link to v3: https://lore.kernel.org/r/20230314-doc-checkpatch-closes-tag-v3-0-d1bdcf31c71c@tessares.net
> 
> Changes in v3:
> - Patch 1/4 now allow using the "Closes" tag with any kind of bug
>   reports, as long as the link is public. (Thorsten)
> - The former patch 2/2 has been split in two: first to use a list for
>   the different "link" tags (Joe). Then to allow the 'Closes' tag.
> - A new patch has been added to let checkpatch.pl checking if "Closes"
>   and "Links" are used with a URL.
> - Link to v2: https://lore.kernel.org/r/20230314-doc-checkpatch-closes-tag-v2-0-f4a417861f6d@tessares.net
> 
> Changes in v2:
> - The text on patch 1/2 has been reworked thanks to Jon, Bagas and
>   Thorsten. See the individual changelog on the patch for more details.
> - Private bug trackers and invalid URLs are clearly marked as forbidden
>   to avoid being misused. (Linus)
> - Rebased on top of Linus' repo.
> - Link to v1: https://lore.kernel.org/r/20230314-doc-checkpatch-closes-tag-v1-0-1b83072e9a9a@tessares.net
> 
> ---
> Matthieu Baerts (5):
>       docs: process: allow Closes tags with links
>       checkpatch: don't print the next line if not defined
>       checkpatch: use a list of "link" tags
>       checkpatch: allow Closes tags with links
>       checkpatch: check for misuse of the link tags
> 
>  Documentation/process/5.Posting.rst          | 22 ++++++++++----
>  Documentation/process/submitting-patches.rst | 26 +++++++++++------
>  scripts/checkpatch.pl                        | 43 ++++++++++++++++++++++------
>  3 files changed, 70 insertions(+), 21 deletions(-)
> ---
> base-commit: 7e364e56293bb98cae1b55fd835f5991c4e96e7d
> change-id: 20230314-doc-checkpatch-closes-tag-1731b57556b1
> 
> Best regards,


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: checkpatch: check for misuse of the link tags: Tests Results
  2023-04-03 16:23   ` Matthieu Baerts
  (?)
@ 2023-04-03 18:04   ` MPTCP CI
  -1 siblings, 0 replies; 19+ messages in thread
From: MPTCP CI @ 2023-04-03 18:04 UTC (permalink / raw)
  To: Matthieu Baerts; +Cc: mptcp

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6018756900552704
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6018756900552704/summary/summary.txt

- KVM Validation: normal (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5455806947131392
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5455806947131392/summary/summary.txt

- KVM Validation: debug (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6581706853974016
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6581706853974016/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Unstable: 1 failed test(s): selftest_mptcp_join 🔴:
  - Task: https://cirrus-ci.com/task/4752119505354752
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4752119505354752/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/5055c32bd437


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (Tessares)

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v4 1/5] docs: process: allow Closes tags with links
  2023-04-03 16:23   ` Matthieu Baerts
@ 2023-04-04  8:09     ` Thorsten Leemhuis
  -1 siblings, 0 replies; 19+ messages in thread
From: Thorsten Leemhuis @ 2023-04-04  8:09 UTC (permalink / raw)
  To: Matthieu Baerts, Jonathan Corbet, Andy Whitcroft, Joe Perches,
	Dwaipayan Ray, Lukas Bulwahn, Kai Wasserbäch, Andrew Morton,
	David Airlie, Daniel Vetter, Konstantin Ryabitsev, Bagas Sanjaya,
	Linus Torvalds
  Cc: linux-doc, linux-kernel, dri-devel, mptcp


On 03.04.23 18:23, Matthieu Baerts wrote:
> [...]
> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> index 828997bc9ff9..12d58ddc2b8a 100644
> --- a/Documentation/process/submitting-patches.rst
> +++ b/Documentation/process/submitting-patches.rst
> @@ -113,11 +113,9 @@ there is no collision with your six-character ID now, that condition may
>  change five years from now.
>  
>  If related discussions or any other background information behind the change
> -can be found on the web, add 'Link:' tags pointing to it. In case your patch
> -fixes a bug, for example, add a tag with a URL referencing the report in the
> -mailing list archives or a bug tracker; if the patch is a result of some
> -earlier mailing list discussion or something documented on the web, point to
> -it.
> +can be found on the web, add 'Link:' tags pointing to it. If the patch is a
> +result of some earlier mailing list discussions or something documented on the
> +web, point to it.
>  
>  When linking to mailing list archives, preferably use the lore.kernel.org
>  message archiver service. To create the link URL, use the contents of the
> @@ -134,6 +132,16 @@ resources. In addition to giving a URL to a mailing list archive or bug,
>  summarize the relevant points of the discussion that led to the
>  patch as submitted.
>  
> +In case your patch fixes a bug, use the 'Closes:' tag with a URL referencing
> +the report in the mailing list archives or a public bug tracker. For example::
> +
> +	Closes: https://example.com/issues/1234

YMMV, but is this...

> +Some bug trackers have the ability to close issues automatically when a
> +commit with such a tag is applied. Some bots monitoring mailing lists can
> +also track such tags and take certain actions. Private bug trackers and
> +invalid URLs are forbidden.
> +

...section (and a similar one in the other document) really worth it
and/or does it have to be that long? A simple "Some bug trackers then
will automatically close the issue when the commit is merged" IMHO would
suffice, but OTOH it might be considered common knowledge. And the
"found on the web", "a public bug tracker" (both quoted above) and
"available on the web" (quoted below) already make it pretty clear that
links to private bug trackers are now desired. And there is also a
"Please check the link to make sure that it is actually working and
points to the relevant message." in submitting-patches.rst already, so
invalid URLs are obviously not wanted either.

>  If your patch fixes a bug in a specific commit, e.g. you found an issue using
>  ``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
>  the SHA-1 ID, and the one line summary.  Do not split the tag across multiple
> @@ -498,9 +506,11 @@ Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:
>  The Reported-by tag gives credit to people who find bugs and report them and it
>  hopefully inspires them to help us again in the future. The tag is intended for
>  bugs; please do not use it to credit feature requests. The tag should be
> -followed by a Link: tag pointing to the report, unless the report is not
> -available on the web. Please note that if the bug was reported in private, then
> -ask for permission first before using the Reported-by tag.
> +followed by a Closes: tag pointing to the report, unless the report is not
> +available on the web. The Link: tag can be used instead of Closes: if the patch
> +fixes a part of the issue(s) being reported. Please note that if the bug was
> +reported in private, then ask for permission first before using the Reported-by
> +tag.
>  
>  A Tested-by: tag indicates that the patch has been successfully tested (in
>  some environment) by the person named.  This tag informs maintainers that

Ciao, Thorsten

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v4 1/5] docs: process: allow Closes tags with links
@ 2023-04-04  8:09     ` Thorsten Leemhuis
  0 siblings, 0 replies; 19+ messages in thread
From: Thorsten Leemhuis @ 2023-04-04  8:09 UTC (permalink / raw)
  To: Matthieu Baerts, Jonathan Corbet, Andy Whitcroft, Joe Perches,
	Dwaipayan Ray, Lukas Bulwahn, Kai Wasserbäch, Andrew Morton,
	David Airlie, Daniel Vetter, Konstantin Ryabitsev, Bagas Sanjaya,
	Linus Torvalds
  Cc: mptcp, linux-kernel, dri-devel, linux-doc


On 03.04.23 18:23, Matthieu Baerts wrote:
> [...]
> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> index 828997bc9ff9..12d58ddc2b8a 100644
> --- a/Documentation/process/submitting-patches.rst
> +++ b/Documentation/process/submitting-patches.rst
> @@ -113,11 +113,9 @@ there is no collision with your six-character ID now, that condition may
>  change five years from now.
>  
>  If related discussions or any other background information behind the change
> -can be found on the web, add 'Link:' tags pointing to it. In case your patch
> -fixes a bug, for example, add a tag with a URL referencing the report in the
> -mailing list archives or a bug tracker; if the patch is a result of some
> -earlier mailing list discussion or something documented on the web, point to
> -it.
> +can be found on the web, add 'Link:' tags pointing to it. If the patch is a
> +result of some earlier mailing list discussions or something documented on the
> +web, point to it.
>  
>  When linking to mailing list archives, preferably use the lore.kernel.org
>  message archiver service. To create the link URL, use the contents of the
> @@ -134,6 +132,16 @@ resources. In addition to giving a URL to a mailing list archive or bug,
>  summarize the relevant points of the discussion that led to the
>  patch as submitted.
>  
> +In case your patch fixes a bug, use the 'Closes:' tag with a URL referencing
> +the report in the mailing list archives or a public bug tracker. For example::
> +
> +	Closes: https://example.com/issues/1234

YMMV, but is this...

> +Some bug trackers have the ability to close issues automatically when a
> +commit with such a tag is applied. Some bots monitoring mailing lists can
> +also track such tags and take certain actions. Private bug trackers and
> +invalid URLs are forbidden.
> +

...section (and a similar one in the other document) really worth it
and/or does it have to be that long? A simple "Some bug trackers then
will automatically close the issue when the commit is merged" IMHO would
suffice, but OTOH it might be considered common knowledge. And the
"found on the web", "a public bug tracker" (both quoted above) and
"available on the web" (quoted below) already make it pretty clear that
links to private bug trackers are now desired. And there is also a
"Please check the link to make sure that it is actually working and
points to the relevant message." in submitting-patches.rst already, so
invalid URLs are obviously not wanted either.

>  If your patch fixes a bug in a specific commit, e.g. you found an issue using
>  ``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
>  the SHA-1 ID, and the one line summary.  Do not split the tag across multiple
> @@ -498,9 +506,11 @@ Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:
>  The Reported-by tag gives credit to people who find bugs and report them and it
>  hopefully inspires them to help us again in the future. The tag is intended for
>  bugs; please do not use it to credit feature requests. The tag should be
> -followed by a Link: tag pointing to the report, unless the report is not
> -available on the web. Please note that if the bug was reported in private, then
> -ask for permission first before using the Reported-by tag.
> +followed by a Closes: tag pointing to the report, unless the report is not
> +available on the web. The Link: tag can be used instead of Closes: if the patch
> +fixes a part of the issue(s) being reported. Please note that if the bug was
> +reported in private, then ask for permission first before using the Reported-by
> +tag.
>  
>  A Tested-by: tag indicates that the patch has been successfully tested (in
>  some environment) by the person named.  This tag informs maintainers that

Ciao, Thorsten

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v4 1/5] docs: process: allow Closes tags with links
  2023-04-04  8:09     ` Thorsten Leemhuis
@ 2023-04-04  8:42       ` Matthieu Baerts
  -1 siblings, 0 replies; 19+ messages in thread
From: Matthieu Baerts @ 2023-04-04  8:42 UTC (permalink / raw)
  To: Thorsten Leemhuis, Jonathan Corbet, Andy Whitcroft, Joe Perches,
	Dwaipayan Ray, Lukas Bulwahn, Kai Wasserbäch, Andrew Morton,
	David Airlie, Daniel Vetter, Konstantin Ryabitsev, Bagas Sanjaya,
	Linus Torvalds
  Cc: linux-doc, linux-kernel, dri-devel, mptcp

Hi Thorsten,

Thank you for this review.

On 04/04/2023 10:09, Thorsten Leemhuis wrote:
> 
> On 03.04.23 18:23, Matthieu Baerts wrote:
>> [...]
>> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
>> index 828997bc9ff9..12d58ddc2b8a 100644
>> --- a/Documentation/process/submitting-patches.rst
>> +++ b/Documentation/process/submitting-patches.rst
>> @@ -113,11 +113,9 @@ there is no collision with your six-character ID now, that condition may
>>  change five years from now.
>>  
>>  If related discussions or any other background information behind the change
>> -can be found on the web, add 'Link:' tags pointing to it. In case your patch
>> -fixes a bug, for example, add a tag with a URL referencing the report in the
>> -mailing list archives or a bug tracker; if the patch is a result of some
>> -earlier mailing list discussion or something documented on the web, point to
>> -it.
>> +can be found on the web, add 'Link:' tags pointing to it. If the patch is a
>> +result of some earlier mailing list discussions or something documented on the
>> +web, point to it.
>>  
>>  When linking to mailing list archives, preferably use the lore.kernel.org
>>  message archiver service. To create the link URL, use the contents of the
>> @@ -134,6 +132,16 @@ resources. In addition to giving a URL to a mailing list archive or bug,
>>  summarize the relevant points of the discussion that led to the
>>  patch as submitted.
>>  
>> +In case your patch fixes a bug, use the 'Closes:' tag with a URL referencing
>> +the report in the mailing list archives or a public bug tracker. For example::
>> +
>> +	Closes: https://example.com/issues/1234
> 
> YMMV, but is this...
> 
>> +Some bug trackers have the ability to close issues automatically when a
>> +commit with such a tag is applied. Some bots monitoring mailing lists can
>> +also track such tags and take certain actions. Private bug trackers and
>> +invalid URLs are forbidden.
>> +
> 
> ...section (and a similar one in the other document) really worth it
> and/or does it have to be that long? A simple "Some bug trackers then
> will automatically close the issue when the commit is merged" IMHO would
> suffice, but OTOH it might be considered common knowledge. And the
> "found on the web", "a public bug tracker" (both quoted above) and
> "available on the web" (quoted below) already make it pretty clear that
> links to private bug trackers are now desired. And there is also a
> "Please check the link to make sure that it is actually working and
> points to the relevant message." in submitting-patches.rst already, so
> invalid URLs are obviously not wanted either.

This paragraph seems worth it to me: the two first sentences explain how
this tag can be used by external tools and the last one clearly explain
what is not allowed. I agree that it makes sense and it is somehow
already described around with the "positive form" but it is very common
to use the "Closes:" tag with just the ticket ID, not the full URL. It
might then be important to clearly mention that it has to be used with a
valid URL and not a short version. While at it, I think it is fine to
add that private bug trackers are forbidden too because it can be very
tempting for devs to use them if automations are in place. And also
because checkpatch.pl is not going to verify if URLs are public.

But I'm clearly not an expert in writing docs, it is just my point of
view as a developer :)
I don't mind changing the text.

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v4 1/5] docs: process: allow Closes tags with links
@ 2023-04-04  8:42       ` Matthieu Baerts
  0 siblings, 0 replies; 19+ messages in thread
From: Matthieu Baerts @ 2023-04-04  8:42 UTC (permalink / raw)
  To: Thorsten Leemhuis, Jonathan Corbet, Andy Whitcroft, Joe Perches,
	Dwaipayan Ray, Lukas Bulwahn, Kai Wasserbäch, Andrew Morton,
	David Airlie, Daniel Vetter, Konstantin Ryabitsev, Bagas Sanjaya,
	Linus Torvalds
  Cc: mptcp, linux-kernel, dri-devel, linux-doc

Hi Thorsten,

Thank you for this review.

On 04/04/2023 10:09, Thorsten Leemhuis wrote:
> 
> On 03.04.23 18:23, Matthieu Baerts wrote:
>> [...]
>> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
>> index 828997bc9ff9..12d58ddc2b8a 100644
>> --- a/Documentation/process/submitting-patches.rst
>> +++ b/Documentation/process/submitting-patches.rst
>> @@ -113,11 +113,9 @@ there is no collision with your six-character ID now, that condition may
>>  change five years from now.
>>  
>>  If related discussions or any other background information behind the change
>> -can be found on the web, add 'Link:' tags pointing to it. In case your patch
>> -fixes a bug, for example, add a tag with a URL referencing the report in the
>> -mailing list archives or a bug tracker; if the patch is a result of some
>> -earlier mailing list discussion or something documented on the web, point to
>> -it.
>> +can be found on the web, add 'Link:' tags pointing to it. If the patch is a
>> +result of some earlier mailing list discussions or something documented on the
>> +web, point to it.
>>  
>>  When linking to mailing list archives, preferably use the lore.kernel.org
>>  message archiver service. To create the link URL, use the contents of the
>> @@ -134,6 +132,16 @@ resources. In addition to giving a URL to a mailing list archive or bug,
>>  summarize the relevant points of the discussion that led to the
>>  patch as submitted.
>>  
>> +In case your patch fixes a bug, use the 'Closes:' tag with a URL referencing
>> +the report in the mailing list archives or a public bug tracker. For example::
>> +
>> +	Closes: https://example.com/issues/1234
> 
> YMMV, but is this...
> 
>> +Some bug trackers have the ability to close issues automatically when a
>> +commit with such a tag is applied. Some bots monitoring mailing lists can
>> +also track such tags and take certain actions. Private bug trackers and
>> +invalid URLs are forbidden.
>> +
> 
> ...section (and a similar one in the other document) really worth it
> and/or does it have to be that long? A simple "Some bug trackers then
> will automatically close the issue when the commit is merged" IMHO would
> suffice, but OTOH it might be considered common knowledge. And the
> "found on the web", "a public bug tracker" (both quoted above) and
> "available on the web" (quoted below) already make it pretty clear that
> links to private bug trackers are now desired. And there is also a
> "Please check the link to make sure that it is actually working and
> points to the relevant message." in submitting-patches.rst already, so
> invalid URLs are obviously not wanted either.

This paragraph seems worth it to me: the two first sentences explain how
this tag can be used by external tools and the last one clearly explain
what is not allowed. I agree that it makes sense and it is somehow
already described around with the "positive form" but it is very common
to use the "Closes:" tag with just the ticket ID, not the full URL. It
might then be important to clearly mention that it has to be used with a
valid URL and not a short version. While at it, I think it is fine to
add that private bug trackers are forbidden too because it can be very
tempting for devs to use them if automations are in place. And also
because checkpatch.pl is not going to verify if URLs are public.

But I'm clearly not an expert in writing docs, it is just my point of
view as a developer :)
I don't mind changing the text.

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2023-04-04  8:42 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-03 16:23 [PATCH v4 0/5] docs & checkpatch: allow Closes tags with links Matthieu Baerts
2023-04-03 16:23 ` Matthieu Baerts
2023-04-03 16:23 ` [PATCH v4 1/5] docs: process: " Matthieu Baerts
2023-04-03 16:23   ` Matthieu Baerts
2023-04-04  8:09   ` Thorsten Leemhuis
2023-04-04  8:09     ` Thorsten Leemhuis
2023-04-04  8:42     ` Matthieu Baerts
2023-04-04  8:42       ` Matthieu Baerts
2023-04-03 16:23 ` [PATCH v4 2/5] checkpatch: don't print the next line if not defined Matthieu Baerts
2023-04-03 16:23   ` Matthieu Baerts
2023-04-03 16:23 ` [PATCH v4 3/5] checkpatch: use a list of "link" tags Matthieu Baerts
2023-04-03 16:23   ` Matthieu Baerts
2023-04-03 16:23 ` [PATCH v4 4/5] checkpatch: allow Closes tags with links Matthieu Baerts
2023-04-03 16:23   ` Matthieu Baerts
2023-04-03 16:23 ` [PATCH v4 5/5] checkpatch: check for misuse of the link tags Matthieu Baerts
2023-04-03 16:23   ` Matthieu Baerts
2023-04-03 18:04   ` checkpatch: check for misuse of the link tags: Tests Results MPTCP CI
2023-04-03 16:44 ` [PATCH v4 0/5] docs & checkpatch: allow Closes tags with links Joe Perches
2023-04-03 16:44   ` Joe Perches

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.