linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v1 0/1] Create 'Reported:' and 'Reviewed:' tags for links in commit messages
@ 2021-11-22  7:33 Thorsten Leemhuis
  2021-11-22  7:33 ` [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:' Thorsten Leemhuis
  2021-11-22 15:12 ` [RFC PATCH v1 0/1] Create 'Reported:' and 'Reviewed:' tags for links in commit messages Konstantin Ryabitsev
  0 siblings, 2 replies; 32+ messages in thread
From: Thorsten Leemhuis @ 2021-11-22  7:33 UTC (permalink / raw)
  To: workflows
  Cc: Linux Kernel Mailing List, Konstantin Ryabitsev, Jonathan Corbet

The following patch proposes to create two new tags for mentioning links in
commit messages. They are meant to make it obvious what provided links are
about, which is useful for both users and scripts studying commits.

The new tag 'Reported:' is meant to be used for linking to bug reports, while
'Reviewed:' should point to the last review of the patch in question. They
supplement 'Link:', which until now has been used for these two as well as other
purposes; it stays around for the latter use case, for example for links to PDFs
or webpages with background information relevant for the patch.

This submission partly got indirectly triggered by regzbot, my Linux kernel
regression tracking bot (https://linux-regtracking.leemhuis.info/regzbot/ ). It
automatically marks a tracked regression as resolved when it notices a commit
with a 'Link:' pointing to the report of the tracked regression. In preparation
for this I recently improved the kernel's documentation on 'Link:' to the best
of my understanding in commit 1f57bd42b77c
(https://git.kernel.org/linus/1f57bd42b77c ). I also started pointing out that
usage to various people when I noticed the links were missing. Some didn't know
that 'Link:' was supposed to be like this, while developers from the DRM
subsystem were using 'References:' instead; some developer also simply used
footnotes style and there are also quite a few developers unaware they are
supposed to add links to bug reports.

I could continue down that path and further educating developers, no big deal.
But I wondered if I was making a problem worse, as I always found it a bit
confusing that 'Link:' is used for different purposes and thus ambiguous. The
situation thus made me wonder if this wouldn't be a good time to improve the
whole situation by going a step further. That's how the proposed patch (still a
bit rough) came to light.

Obviously such a change will force developers and maintainers to adjust, so it's
nothing that should be done lighthearted. But I guess in the long-run it's worth
it. And for 'Review:' the conversion shouldn't be much work for people, as many
just need to update their `git am` hook or switch to a hypothetical new version
of b4 that was adjusted to place 'Reviewed:' tags instead of 'Link:'. It's a bit
more of a hassle when it comes to 'Reported:', as some people will need to
update their muscle memory. But the similarity to the 'Reported-by:' tag (to be
used in the same situation) should help here; and quite a bit of education in
this area is needed anyway (see above).

In both use cases there is no real harm done if it takes the world a while to
adapt, as 'Link:' stays around.

I won't mind at all if this bold move gets rejected, if that's the case I'll
simply modify the patch a bit to properly describe the 'Link:' tag in
Documentation/process/5.Posting.rst, as it's not mentioning it at all right now.
But I think proposing this was worth a shot, as having distant tags for various
types of links might be handy for other purposes in the long-run as well.

Ciao, Thorsten

P.S.: I'm sending v1/RFC to workflows list and LKML only to test the waters and
hopefully collect some supportive arguments. If this doesn't get shot down there
I'll post a v2/RFC to wider audience including Linus and Greg; not sure yet if
I'll to Cc ksummit then as well, as it would be abusing the list somewhat, but
OTOH is a good way to reach a lot of core people that might care about this.

Thorsten Leemhuis (1):
  docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'

 Documentation/maintainer/configure-git.rst   |  6 +--
 Documentation/process/5.Posting.rst          | 54 ++++++++++++++------
 Documentation/process/submitting-patches.rst | 22 ++++----
 3 files changed, 53 insertions(+), 29 deletions(-)


base-commit: b96ff02ab2be1791248237b1bf318aaf62e8b701
-- 
2.31.1


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

* [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-22  7:33 [RFC PATCH v1 0/1] Create 'Reported:' and 'Reviewed:' tags for links in commit messages Thorsten Leemhuis
@ 2021-11-22  7:33 ` Thorsten Leemhuis
  2021-11-22 16:29   ` Steven Rostedt
                     ` (2 more replies)
  2021-11-22 15:12 ` [RFC PATCH v1 0/1] Create 'Reported:' and 'Reviewed:' tags for links in commit messages Konstantin Ryabitsev
  1 sibling, 3 replies; 32+ messages in thread
From: Thorsten Leemhuis @ 2021-11-22  7:33 UTC (permalink / raw)
  To: workflows
  Cc: Linux Kernel Mailing List, Konstantin Ryabitsev, Jonathan Corbet

Introduce the tags 'Reported:' and 'Reviewed:' in addition to 'Link:',
as the latter is overloaded and hence doesn't indicate what the provided
URL is about. Documenting these also provides clarity, as a few
developers have used 'References:' to point to problem reports;
nevertheless 'Reported:' was chosen for this purpose, as it perfectly
matches up with the 'Reported-by:' tag commonly used already and needed
in this situation already.

Signed-off-by: Thorsten Leemhuis <linux@leemhuis.info>
To: workflows@vger.kernel.org
---
v1/RFC:
- first, *rough version* to see how this idea is received in the
  community
---
 Documentation/maintainer/configure-git.rst   |  6 +--
 Documentation/process/5.Posting.rst          | 54 ++++++++++++++------
 Documentation/process/submitting-patches.rst | 22 ++++----
 3 files changed, 53 insertions(+), 29 deletions(-)

diff --git a/Documentation/maintainer/configure-git.rst b/Documentation/maintainer/configure-git.rst
index 80ae5030a590..8429d45d661c 100644
--- a/Documentation/maintainer/configure-git.rst
+++ b/Documentation/maintainer/configure-git.rst
@@ -40,12 +40,12 @@ Creating commit links to lore.kernel.org
 The web site http://lore.kernel.org is meant as a grand archive of all mail
 list traffic concerning or influencing the kernel development. Storing archives
 of patches here is a recommended practice, and when a maintainer applies a
-patch to a subsystem tree, it is a good idea to provide a Link: tag with a
+patch to a subsystem tree, it is a good idea to provide a Reviewed: tag with a
 reference back to the lore archive so that people that browse the commit
 history can find related discussions and rationale behind a certain change.
 The link tag will look like this:
 
-    Link: https://lore.kernel.org/r/<message-id>
+    Reviewed: https://lore.kernel.org/r/<message-id>
 
 This can be configured to happen automatically any time you issue ``git am``
 by adding the following hook into your git:
@@ -56,7 +56,7 @@ by adding the following hook into your git:
 	$ cat >.git/hooks/applypatch-msg <<'EOF'
 	#!/bin/sh
 	. git-sh-setup
-	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|g;' "$1"
+	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Reviewed: https://lore.kernel.org/r/$1|g;' "$1"
 	test -x "$GIT_DIR/hooks/commit-msg" &&
 		exec "$GIT_DIR/hooks/commit-msg" ${1+"$@"}
 	:
diff --git a/Documentation/process/5.Posting.rst b/Documentation/process/5.Posting.rst
index 855a70b80269..c5d7c982a373 100644
--- a/Documentation/process/5.Posting.rst
+++ b/Documentation/process/5.Posting.rst
@@ -197,53 +197,75 @@ the build process, for example, or editor backup files) in the patch.  The
 file "dontdiff" in the Documentation directory can help in this regard;
 pass it to diff with the "-X" option.
 
-The tags mentioned above are used to describe how various developers have
-been associated with the development of this patch.  They are described in
-detail in
-the :ref:`Documentation/process/submitting-patches.rst <submittingpatches>`
-document; what follows here is a brief summary.  Each of these lines has
-the format:
+The tags already briefly mentioned above are used to provide insights how
+the patch came into being. They are described in detail in the
+:ref:`Documentation/process/submitting-patches.rst <submittingpatches>`
+document; what follows here is a brief summary.
 
-::
+One tag is used to refer to earlier commits which had problems fixed by
+the patch::
+
+	Fixes: 1f2e3d4c5b6a ("The first line of the commit specified by the first 12 characters of its SHA-1 ID")
+
+A second kind of tags is used to link webpages with additional details. There
+are three different tags of this sort which all use the following format::
+
+	Reported: https://example.com/somewhere.html  optional-other-stuff
+
+The tags in common use are:
+
+ - ``Reported:`` points to a report of a problem fixed by this patch. The
+   provided URL thus might point to a entry in a bug tracker or a mail in a
+   mailing list archive. Typically this tag is followed by a "Reported-by:"
+   tag (see below).
+
+ - ``Link:`` points to websites providing additional backgrounds or details,
+   for example a document with a specification implemented by the patch.
+
+ - ``Reviewed:`` ignore this, as maintainers add it when applying a patch, to
+   make the commit point to the latest public review of the patch.
+
+A third kind of tags are used to document which developers were involved in
+the development of the patch. Each of these uses this format::
 
 	tag: Full Name <email address>  optional-other-stuff
 
 The tags in common use are:
 
- - Signed-off-by: this is a developer's certification that he or she has
+ - ``Signed-off-by:`` is a developer's certification that he or she has
    the right to submit the patch for inclusion into the kernel.  It is an
    agreement to the Developer's Certificate of Origin, the full text of
    which can be found in :ref:`Documentation/process/submitting-patches.rst <submittingpatches>`
    Code without a proper signoff cannot be merged into the mainline.
 
- - Co-developed-by: states that the patch was co-created by several developers;
+ - ``Co-developed-by:`` states that the patch was co-created by several developers;
    it is a used to give attribution to co-authors (in addition to the author
    attributed by the From: tag) when multiple people work on a single patch.
    Every Co-developed-by: must be immediately followed by a Signed-off-by: of
    the associated co-author.  Details and examples can be found in
    :ref:`Documentation/process/submitting-patches.rst <submittingpatches>`.
 
- - Acked-by: indicates an agreement by another developer (often a
+ - ``Acked-by:`` indicates an agreement by another developer (often a
    maintainer of the relevant code) that the patch is appropriate for
    inclusion into the kernel.
 
- - Tested-by: states that the named person has tested the patch and found
+ - ``Tested-by:`` states that the named person has tested the patch and found
    it to work.
 
- - Reviewed-by: the named developer has reviewed the patch for correctness;
+ - ``Reviewed-by:`` the named developer has reviewed the patch for correctness;
    see the reviewer's statement in :ref:`Documentation/process/submitting-patches.rst <submittingpatches>`
    for more detail.
 
- - Reported-by: names a user who reported a problem which is fixed by this
+ - ``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.
 
- - Cc: the named person received a copy of the patch and had the
+ - ``Cc:`` the named person received a copy of the patch and had the
    opportunity to comment on it.
 
-Be careful in the addition of tags to your patches: only Cc: is appropriate
-for addition without the explicit permission of the person named.
+Be careful when adding this sort of tags to your patches: only Cc: is
+appropriate for addition without the explicit permission of the person named.
 
 
 Sending the patch
diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
index da085d63af9b..b7c9155da9bd 100644
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -112,20 +112,22 @@ collisions with shorter IDs a real possibility.  Bear in mind that, even if
 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.
+Add tags linking to any related discussions or background information behind
+the change on the web. For example, if your patch fixes a bug, add a
+`Reported:` tag pointing to the report in the mailing list archives or a bug
+tracker::
 
-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
-``Message-Id`` header of the message without the surrounding angle brackets.
-For example::
+    Reported: https://lore.kernel.org/r/30th.anniversary.repost@klaava.Helsinki.FI/
+
+If the patch is a related to some earlier mailing list discussion or something
+documented on the web, point to it using a `Link:` tag::
 
     Link: https://lore.kernel.org/r/30th.anniversary.repost@klaava.Helsinki.FI/
 
+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
+``Message-Id`` header of the message without the surrounding angle brackets,
+e.g. "30th.anniversary.repost@klaava.Helsinki.FI" in the two examples above.
 Please check the link to make sure that it is actually working and points
 to the relevant message.
 
-- 
2.31.1


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

* Re: [RFC PATCH v1 0/1] Create 'Reported:' and 'Reviewed:' tags for links in commit messages
  2021-11-22  7:33 [RFC PATCH v1 0/1] Create 'Reported:' and 'Reviewed:' tags for links in commit messages Thorsten Leemhuis
  2021-11-22  7:33 ` [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:' Thorsten Leemhuis
@ 2021-11-22 15:12 ` Konstantin Ryabitsev
  2021-11-22 17:04   ` Steven Rostedt
  1 sibling, 1 reply; 32+ messages in thread
From: Konstantin Ryabitsev @ 2021-11-22 15:12 UTC (permalink / raw)
  To: Thorsten Leemhuis; +Cc: workflows, Linux Kernel Mailing List, Jonathan Corbet

On Mon, Nov 22, 2021 at 08:33:41AM +0100, Thorsten Leemhuis wrote:
> The following patch proposes to create two new tags for mentioning links in
> commit messages. They are meant to make it obvious what provided links are
> about, which is useful for both users and scripts studying commits.
> 
> The new tag 'Reported:' is meant to be used for linking to bug reports, while
> 'Reviewed:' should point to the last review of the patch in question. They
> supplement 'Link:', which until now has been used for these two as well as other
> purposes; it stays around for the latter use case, for example for links to PDFs
> or webpages with background information relevant for the patch.

As an alternative, I can offer that we continue to use Link: trailers with
extra data following the hashtag, as is already done for other trailers:

    Link: https://bugzilla.kernel.org/show_bug.cgi?id=215101   #report
    Link: https://lore.kernel.org/r/fobarbaz.5551212@localhost #review

Note, that this merely for completeness, not in opposition to the proposal. I
find the "Link:" trailer to be semantically redundant, since what follows is
already clearly a hyperlink. Adding "Link: " in front of it is only necessary
for consistency and machine parsing reasons.

Regards,
-K

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-22  7:33 ` [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:' Thorsten Leemhuis
@ 2021-11-22 16:29   ` Steven Rostedt
  2021-11-22 18:50     ` Thorsten Leemhuis
  2021-11-23 18:52   ` Eric Wong
  2021-11-29 22:16   ` Jonathan Corbet
  2 siblings, 1 reply; 32+ messages in thread
From: Steven Rostedt @ 2021-11-22 16:29 UTC (permalink / raw)
  To: Thorsten Leemhuis
  Cc: workflows, Linux Kernel Mailing List, Konstantin Ryabitsev,
	Jonathan Corbet

On Mon, 22 Nov 2021 08:33:42 +0100
Thorsten Leemhuis <linux@leemhuis.info> wrote:

> Introduce the tags 'Reported:' and 'Reviewed:' in addition to 'Link:',
> as the latter is overloaded and hence doesn't indicate what the provided
> URL is about. Documenting these also provides clarity, as a few
> developers have used 'References:' to point to problem reports;
> nevertheless 'Reported:' was chosen for this purpose, as it perfectly
> matches up with the 'Reported-by:' tag commonly used already and needed
> in this situation already.

I like the differences between "Reorted:" and "Reviewed:", although I may
keep the "Link" instead of Reviewed because my automate scripts just give
the link of the patch, and there's seldom a review attached to it :-/

That said, I would like a way to have versions show a link to the last
version that was reviewed.

v1: has no tags

v2: has a Reviewed: tag to v1.

v3: has a Reviewed: tag to v2

[...]

Then the final commit could have a "Link" or "Reviewed" tag to v3, even
though there may not be any reviews to v3, but v3 has the link to v2, and
v2 has the link to v1, etc.

-- Steve

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

* Re: [RFC PATCH v1 0/1] Create 'Reported:' and 'Reviewed:' tags for links in commit messages
  2021-11-22 15:12 ` [RFC PATCH v1 0/1] Create 'Reported:' and 'Reviewed:' tags for links in commit messages Konstantin Ryabitsev
@ 2021-11-22 17:04   ` Steven Rostedt
  2021-11-22 18:40     ` Thorsten Leemhuis
  0 siblings, 1 reply; 32+ messages in thread
From: Steven Rostedt @ 2021-11-22 17:04 UTC (permalink / raw)
  To: Konstantin Ryabitsev
  Cc: Thorsten Leemhuis, workflows, Linux Kernel Mailing List, Jonathan Corbet

On Mon, 22 Nov 2021 10:12:33 -0500
Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:

> As an alternative, I can offer that we continue to use Link: trailers with
> extra data following the hashtag, as is already done for other trailers:
> 
>     Link: https://bugzilla.kernel.org/show_bug.cgi?id=215101   #report
>     Link: https://lore.kernel.org/r/fobarbaz.5551212@localhost #review
> 
> Note, that this merely for completeness, not in opposition to the proposal. I
> find the "Link:" trailer to be semantically redundant, since what follows is
> already clearly a hyperlink. Adding "Link: " in front of it is only necessary
> for consistency and machine parsing reasons.

Machine parsing is the main reason for the Link: tag. I have scripts that
key off of that tag and ignore any other "http" reference.

Perhaps the above is better, as it means we don't need to update our
scripts for that parsing.

-- Steve

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

* Re: [RFC PATCH v1 0/1] Create 'Reported:' and 'Reviewed:' tags for links in commit messages
  2021-11-22 17:04   ` Steven Rostedt
@ 2021-11-22 18:40     ` Thorsten Leemhuis
  2021-11-22 18:48       ` Steven Rostedt
  0 siblings, 1 reply; 32+ messages in thread
From: Thorsten Leemhuis @ 2021-11-22 18:40 UTC (permalink / raw)
  To: Steven Rostedt, Konstantin Ryabitsev
  Cc: workflows, Linux Kernel Mailing List, Jonathan Corbet



On 22.11.21 18:04, Steven Rostedt wrote:
> On Mon, 22 Nov 2021 10:12:33 -0500
> Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> 
>> As an alternative, I can offer that we continue to use Link: trailers with
>> extra data following the hashtag, as is already done for other trailers:
>>
>>     Link: https://bugzilla.kernel.org/show_bug.cgi?id=215101   #report
>>     Link: https://lore.kernel.org/r/fobarbaz.5551212@localhost #review
>>
>> Note, that this merely for completeness, not in opposition to the proposal.

Thx, I'll mention those as a alternative in v2

>> I
>> find the "Link:" trailer to be semantically redundant, since what follows is
>> already clearly a hyperlink. Adding "Link: " in front of it is only necessary
>> for consistency and machine parsing reasons.
> 
> Machine parsing is the main reason for the Link: tag. I have scripts that
> key off of that tag and ignore any other "http" reference.
> 
> Perhaps the above is better, as it means we don't need to update our
> scripts for that parsing.

Hmmm. I'm not opposed, but I wonder if 'Reported:' and 'Reviewed:' are
this tiny bit easier to handle (both for placing and analyzing scripts)
that makes the difference between "nice idea, but fails to be used in
the field" and "after some tradition phase this becomes the new normal"
in the end.

Whatever, will mention that and point to this post in the next round.
Thx for the feedback!

Ciao, Thorsten

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

* Re: [RFC PATCH v1 0/1] Create 'Reported:' and 'Reviewed:' tags for links in commit messages
  2021-11-22 18:40     ` Thorsten Leemhuis
@ 2021-11-22 18:48       ` Steven Rostedt
  0 siblings, 0 replies; 32+ messages in thread
From: Steven Rostedt @ 2021-11-22 18:48 UTC (permalink / raw)
  To: Thorsten Leemhuis
  Cc: Konstantin Ryabitsev, workflows, Linux Kernel Mailing List,
	Jonathan Corbet

On Mon, 22 Nov 2021 19:40:46 +0100
Thorsten Leemhuis <linux@leemhuis.info> wrote:

> Hmmm. I'm not opposed, but I wonder if 'Reported:' and 'Reviewed:' are
> this tiny bit easier to handle (both for placing and analyzing scripts)
> that makes the difference between "nice idea, but fails to be used in
> the field" and "after some tradition phase this becomes the new normal"
> in the end.

Either way is fine for me. My scripts are trivial and easy to update. I'm
more worried about what other people may use.

-- Steve

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-22 16:29   ` Steven Rostedt
@ 2021-11-22 18:50     ` Thorsten Leemhuis
  2021-11-22 20:24       ` Steven Rostedt
  0 siblings, 1 reply; 32+ messages in thread
From: Thorsten Leemhuis @ 2021-11-22 18:50 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: workflows, Linux Kernel Mailing List, Konstantin Ryabitsev,
	Jonathan Corbet

On 22.11.21 17:29, Steven Rostedt wrote:
> On Mon, 22 Nov 2021 08:33:42 +0100
> Thorsten Leemhuis <linux@leemhuis.info> wrote:
> 
>> Introduce the tags 'Reported:' and 'Reviewed:' in addition to 'Link:',
>> as the latter is overloaded and hence doesn't indicate what the provided
>> URL is about. Documenting these also provides clarity, as a few
>> developers have used 'References:' to point to problem reports;
>> nevertheless 'Reported:' was chosen for this purpose, as it perfectly
>> matches up with the 'Reported-by:' tag commonly used already and needed
>> in this situation already.
> 
> I like the differences between "Reorted:" and "Reviewed:", although I may
> keep the "Link" instead of Reviewed because my automate scripts just give
> the link of the patch, and there's seldom a review attached to it :-/

/me wonders if "Merge Request:" would be a better tag, but at least for
now settles on 'it's nice, as it's close to what people are used from
git forges, but OTOH it somehow feels wrong'

> That said, I would like a way to have versions show a link to the last
> version that was reviewed.
> 
> v1: has no tags
> 
> v2: has a Reviewed: tag to v1.
> 
> v3: has a Reviewed: tag to v2
> 
> [...]
> 
> Then the final commit could have a "Link" or "Reviewed" tag to v3, even
> though there may not be any reviews to v3, but v3 has the link to v2, and
> v2 has the link to v1, etc.

Is that really worth it? Isn't it sufficient if the commit links to the
last public review posting, as that already should link to all earlier
review postings. Sure, not everybody is doing this right now, but maybe
just educating people to do so is better than creating something new.

Ciao, Thorsten

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-22 18:50     ` Thorsten Leemhuis
@ 2021-11-22 20:24       ` Steven Rostedt
  2021-11-23  8:53         ` Thorsten Leemhuis
  0 siblings, 1 reply; 32+ messages in thread
From: Steven Rostedt @ 2021-11-22 20:24 UTC (permalink / raw)
  To: Thorsten Leemhuis
  Cc: workflows, Linux Kernel Mailing List, Konstantin Ryabitsev,
	Jonathan Corbet

On Mon, 22 Nov 2021 19:50:35 +0100
Thorsten Leemhuis <linux@leemhuis.info> wrote:

> > That said, I would like a way to have versions show a link to the last
> > version that was reviewed.
> > 
> > v1: has no tags
> > 
> > v2: has a Reviewed: tag to v1.
> > 
> > v3: has a Reviewed: tag to v2
> > 
> > [...]
> > 
> > Then the final commit could have a "Link" or "Reviewed" tag to v3, even
> > though there may not be any reviews to v3, but v3 has the link to v2, and
> > v2 has the link to v1, etc.  
> 
> Is that really worth it? Isn't it sufficient if the commit links to the
> last public review posting, as that already should link to all earlier
> review postings. Sure, not everybody is doing this right now, but maybe
> just educating people to do so is better than creating something new.

Isn't "as that already should link to all earlier review postings" what I'm
suggesting above? I haven't seen many people do that yet.

-- Steve



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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-22 20:24       ` Steven Rostedt
@ 2021-11-23  8:53         ` Thorsten Leemhuis
  0 siblings, 0 replies; 32+ messages in thread
From: Thorsten Leemhuis @ 2021-11-23  8:53 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: workflows, Linux Kernel Mailing List, Konstantin Ryabitsev,
	Jonathan Corbet



On 22.11.21 21:24, Steven Rostedt wrote:
> On Mon, 22 Nov 2021 19:50:35 +0100
> Thorsten Leemhuis <linux@leemhuis.info> wrote:
> 
>>> That said, I would like a way to have versions show a link to the last
>>> version that was reviewed.
>>>
>>> v1: has no tags
>>>
>>> v2: has a Reviewed: tag to v1.
>>>
>>> v3: has a Reviewed: tag to v2
>>>
>>> [...]
>>>
>>> Then the final commit could have a "Link" or "Reviewed" tag to v3, even
>>> though there may not be any reviews to v3, but v3 has the link to v2, and
>>> v2 has the link to v1, etc.  
>>
>> Is that really worth it? Isn't it sufficient if the commit links to the
>> last public review posting, as that already should link to all earlier
>> review postings. Sure, not everybody is doing this right now, but maybe
>> just educating people to do so is better than creating something new.
> 
> Isn't "as that already should link to all earlier review postings" what I'm
> suggesting above? I haven't seen many people do that yet.

Yeah, you are right, sorry, my perception was wrong.

Any maybe I got your suggestion wrong, but what you suggested sounded to
me like "each patch should link to the previous submission of the
patch". I just wonder if it's way easier and sufficient if just the
cover letter links to the previous or all earlier submissions of the
series in its revision history (sorry, I didn't should have made this
more obvious in my earlier mail); for patches without a cover letter
this obligation obviously would shift to the patch.

IOW: I have something in mind like in this submission:
https://lore.kernel.org/lkml/cover.1637252610.git.sander@svanheule.net/

Only the cover letter links to the earlier version, not the individual
patches.

But I have no strong feeling here, I don't care much about this.

Ciao, Thorsten

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-22  7:33 ` [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:' Thorsten Leemhuis
  2021-11-22 16:29   ` Steven Rostedt
@ 2021-11-23 18:52   ` Eric Wong
  2021-11-24  1:37     ` Junio C Hamano
                       ` (2 more replies)
  2021-11-29 22:16   ` Jonathan Corbet
  2 siblings, 3 replies; 32+ messages in thread
From: Eric Wong @ 2021-11-23 18:52 UTC (permalink / raw)
  To: Thorsten Leemhuis
  Cc: workflows, linux-kernel, Konstantin Ryabitsev, Jonathan Corbet, git

Thorsten Leemhuis <linux@leemhuis.info> wrote:
> diff --git a/Documentation/maintainer/configure-git.rst b/Documentation/maintainer/configure-git.rst
> index 80ae5030a590..8429d45d661c 100644
> --- a/Documentation/maintainer/configure-git.rst
> +++ b/Documentation/maintainer/configure-git.rst

<snip>, +cc git@vger

> @@ -56,7 +56,7 @@ by adding the following hook into your git:
>  	$ cat >.git/hooks/applypatch-msg <<'EOF'
>  	#!/bin/sh
>  	. git-sh-setup
> -	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|g;' "$1"
> +	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Reviewed: https://lore.kernel.org/r/$1|g;' "$1"

Side note: that regexp should match "Message-ID" case-insensitively.
git send-email is an outlier in its capitalization of "Message-Id",
most RFCs capitalize it "Message-ID", as do common MUAs.

git send-email's capitalization does annoy me and I've looked
into changing it; but there's a bunch of tests and probably
dependent code that also need to be updated...

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-23 18:52   ` Eric Wong
@ 2021-11-24  1:37     ` Junio C Hamano
  2021-11-24  6:12       ` Eric Wong
  2021-11-26 12:49       ` Ævar Arnfjörð Bjarmason
  2021-11-24  2:08     ` Ævar Arnfjörð Bjarmason
  2021-11-26  7:29     ` Thorsten Leemhuis
  2 siblings, 2 replies; 32+ messages in thread
From: Junio C Hamano @ 2021-11-24  1:37 UTC (permalink / raw)
  To: Eric Wong
  Cc: Thorsten Leemhuis, workflows, linux-kernel, Konstantin Ryabitsev,
	Jonathan Corbet, git

Eric Wong <e@80x24.org> writes:

> git send-email's capitalization does annoy me and I've looked
> into changing it; but there's a bunch of tests and probably
> dependent code that also need to be updated...

It does annoy me, too, and I do not mind if it gets "fixed", but I
do not know if the cost for vetting a bulk update like that is worth
it.  There is another one outside send-email in log-tree.c that is
responsible for output from format-patch.

Here is the extent of damage in my trial change that seems to pass
the tests locally.

 Documentation/MyFirstContribution.txt | 14 ++++-----
 Documentation/git-format-patch.txt    |  4 +--
 Documentation/git-send-email.txt      |  2 +-
 git-send-email.perl                   |  4 +--
 log-tree.c                            |  2 +-
 mailinfo.c                            |  4 +--
 t/t4014-format-patch.sh               | 56 +++++++++++++++++------------------
 t/t4150-am.sh                         |  8 ++---
 t/t4258/mbox                          |  2 +-
 t/t5100/msg0002                       |  2 +-
 t/t5100/msg0003                       |  2 +-
 t/t5100/msg0012--message-id           |  2 +-
 t/t5100/quoted-cr.mbox                |  4 +--
 t/t5100/sample.mbox                   |  6 ++--
 t/t9001-send-email.sh                 | 38 ++++++++++++------------
 15 files changed, 75 insertions(+), 75 deletions(-)

diff --git i/Documentation/MyFirstContribution.txt w/Documentation/MyFirstContribution.txt
index b20bc8e914..91cb204d52 100644
--- i/Documentation/MyFirstContribution.txt
+++ w/Documentation/MyFirstContribution.txt
@@ -1071,21 +1071,21 @@ between your last version and now, if it's something significant. You do not
 need the exact same body in your second cover letter; focus on explaining to
 reviewers the changes you've made that may not be as visible.
 
-You will also need to go and find the Message-Id of your previous cover letter.
+You will also need to go and find the Message-ID of your previous cover letter.
 You can either note it when you send the first series, from the output of `git
 send-email`, or you can look it up on the
 https://lore.kernel.org/git[mailing list]. Find your cover letter in the
-archives, click on it, then click "permalink" or "raw" to reveal the Message-Id
+archives, click on it, then click "permalink" or "raw" to reveal the Message-ID
 header. It should match:
 
 ----
-Message-Id: <foo.12345.author@example.com>
+Message-ID: <foo.12345.author@example.com>
 ----
 
-Your Message-Id is `<foo.12345.author@example.com>`. This example will be used
-below as well; make sure to replace it with the correct Message-Id for your
-**previous cover letter** - that is, if you're sending v2, use the Message-Id
-from v1; if you're sending v3, use the Message-Id from v2.
+Your Message-ID is `<foo.12345.author@example.com>`. This example will be used
+below as well; make sure to replace it with the correct Message-ID for your
+**previous cover letter** - that is, if you're sending v2, use the Message-ID
+from v1; if you're sending v3, use the Message-ID from v2.
 
 While you're looking at the email, you should also note who is CC'd, as it's
 common practice in the mailing list to keep all CCs on a thread. You can add
diff --git i/Documentation/git-format-patch.txt w/Documentation/git-format-patch.txt
index 113eabc107..daf911f249 100644
--- i/Documentation/git-format-patch.txt
+++ w/Documentation/git-format-patch.txt
@@ -99,7 +99,7 @@ To omit patch numbers from the subject, use `-N`.
 
 If given `--thread`, `git-format-patch` will generate `In-Reply-To` and
 `References` headers to make the second and subsequent patch mails appear
-as replies to the first mail; this also generates a `Message-Id` header to
+as replies to the first mail; this also generates a `Message-ID` header to
 reference.
 
 OPTIONS
@@ -163,7 +163,7 @@ include::diff-options.txt[]
 --no-thread::
 	Controls addition of `In-Reply-To` and `References` headers to
 	make the second and subsequent mails appear as replies to the
-	first.  Also controls generation of the `Message-Id` header to
+	first.  Also controls generation of the `Message-ID` header to
 	reference.
 +
 The optional <style> argument can be either `shallow` or `deep`.
diff --git i/Documentation/git-send-email.txt w/Documentation/git-send-email.txt
index 3db4eab4ba..086f132229 100644
--- i/Documentation/git-send-email.txt
+++ w/Documentation/git-send-email.txt
@@ -91,7 +91,7 @@ See the CONFIGURATION section for `sendemail.multiEdit`.
 
 --in-reply-to=<identifier>::
 	Make the first mail (or all the mails with `--no-thread`) appear as a
-	reply to the given Message-Id, which avoids breaking threads to
+	reply to the given Message-ID, which avoids breaking threads to
 	provide a new patch series.
 	The second and subsequent emails will be sent as replies according to
 	the `--[no-]chain-reply-to` setting.
diff --git i/git-send-email.perl w/git-send-email.perl
index 5262d88ee3..a61134c7d3 100755
--- i/git-send-email.perl
+++ w/git-send-email.perl
@@ -1494,7 +1494,7 @@ sub send_message {
 To: $to${ccline}
 Subject: $subject
 Date: $date
-Message-Id: $message_id
+Message-ID: $message_id
 ";
 	if ($use_xmailer) {
 		$header .= "X-Mailer: git-send-email $gitversion\n";
@@ -1789,7 +1789,7 @@ sub process_file {
 				$has_mime_version = 1;
 				push @xh, $_;
 			}
-			elsif (/^Message-Id: (.*)/i) {
+			elsif (/^Message-ID: (.*)/i) {
 				$message_id = $1;
 			}
 			elsif (/^Content-Transfer-Encoding: (.*)/i) {
diff --git i/log-tree.c w/log-tree.c
index 644893fd8c..818cea5f12 100644
--- i/log-tree.c
+++ w/log-tree.c
@@ -428,7 +428,7 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
 	fprintf(opt->diffopt.file, "From %s Mon Sep 17 00:00:00 2001\n", name);
 	graph_show_oneline(opt->graph);
 	if (opt->message_id) {
-		fprintf(opt->diffopt.file, "Message-Id: <%s>\n", opt->message_id);
+		fprintf(opt->diffopt.file, "Message-ID: <%s>\n", opt->message_id);
 		graph_show_oneline(opt->graph);
 	}
 	if (opt->ref_message_ids && opt->ref_message_ids->nr > 0) {
diff --git i/mailinfo.c w/mailinfo.c
index 02f6f95357..855349cc0e 100644
--- i/mailinfo.c
+++ w/mailinfo.c
@@ -597,7 +597,7 @@ static int check_header(struct mailinfo *mi,
 		ret = 1;
 		goto check_header_out;
 	}
-	if (parse_header(line, "Message-Id", mi, &sb)) {
+	if (parse_header(line, "Message-ID", mi, &sb)) {
 		if (mi->add_message_id)
 			mi->message_id = strbuf_detach(&sb, NULL);
 		ret = 1;
@@ -829,7 +829,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct strbuf *line)
 	if (patchbreak(line)) {
 		if (mi->message_id)
 			strbuf_addf(&mi->log_message,
-				    "Message-Id: %s\n", mi->message_id);
+				    "Message-ID: %s\n", mi->message_id);
 		return 1;
 	}
 
diff --git i/t/t4014-format-patch.sh w/t/t4014-format-patch.sh
index 712d4b5ddf..973899d165 100755
--- i/t/t4014-format-patch.sh
+++ w/t/t4014-format-patch.sh
@@ -445,13 +445,13 @@ test_expect_success 'no threading' '
 
 cat >expect.thread <<EOF
 ---
-Message-Id: <0>
+Message-ID: <0>
 ---
-Message-Id: <1>
+Message-ID: <1>
 In-Reply-To: <0>
 References: <0>
 ---
-Message-Id: <2>
+Message-ID: <2>
 In-Reply-To: <0>
 References: <0>
 EOF
@@ -462,15 +462,15 @@ test_expect_success 'thread' '
 
 cat >expect.in-reply-to <<EOF
 ---
-Message-Id: <0>
+Message-ID: <0>
 In-Reply-To: <1>
 References: <1>
 ---
-Message-Id: <2>
+Message-ID: <2>
 In-Reply-To: <1>
 References: <1>
 ---
-Message-Id: <3>
+Message-ID: <3>
 In-Reply-To: <1>
 References: <1>
 EOF
@@ -482,17 +482,17 @@ test_expect_success 'thread in-reply-to' '
 
 cat >expect.cover-letter <<EOF
 ---
-Message-Id: <0>
+Message-ID: <0>
 ---
-Message-Id: <1>
+Message-ID: <1>
 In-Reply-To: <0>
 References: <0>
 ---
-Message-Id: <2>
+Message-ID: <2>
 In-Reply-To: <0>
 References: <0>
 ---
-Message-Id: <3>
+Message-ID: <3>
 In-Reply-To: <0>
 References: <0>
 EOF
@@ -503,21 +503,21 @@ test_expect_success 'thread cover-letter' '
 
 cat >expect.cl-irt <<EOF
 ---
-Message-Id: <0>
+Message-ID: <0>
 In-Reply-To: <1>
 References: <1>
 ---
-Message-Id: <2>
+Message-ID: <2>
 In-Reply-To: <0>
 References: <1>
 	<0>
 ---
-Message-Id: <3>
+Message-ID: <3>
 In-Reply-To: <0>
 References: <1>
 	<0>
 ---
-Message-Id: <4>
+Message-ID: <4>
 In-Reply-To: <0>
 References: <1>
 	<0>
@@ -535,13 +535,13 @@ test_expect_success 'thread explicit shallow' '
 
 cat >expect.deep <<EOF
 ---
-Message-Id: <0>
+Message-ID: <0>
 ---
-Message-Id: <1>
+Message-ID: <1>
 In-Reply-To: <0>
 References: <0>
 ---
-Message-Id: <2>
+Message-ID: <2>
 In-Reply-To: <1>
 References: <0>
 	<1>
@@ -553,16 +553,16 @@ test_expect_success 'thread deep' '
 
 cat >expect.deep-irt <<EOF
 ---
-Message-Id: <0>
+Message-ID: <0>
 In-Reply-To: <1>
 References: <1>
 ---
-Message-Id: <2>
+Message-ID: <2>
 In-Reply-To: <0>
 References: <1>
 	<0>
 ---
-Message-Id: <3>
+Message-ID: <3>
 In-Reply-To: <2>
 References: <1>
 	<0>
@@ -576,18 +576,18 @@ test_expect_success 'thread deep in-reply-to' '
 
 cat >expect.deep-cl <<EOF
 ---
-Message-Id: <0>
+Message-ID: <0>
 ---
-Message-Id: <1>
+Message-ID: <1>
 In-Reply-To: <0>
 References: <0>
 ---
-Message-Id: <2>
+Message-ID: <2>
 In-Reply-To: <1>
 References: <0>
 	<1>
 ---
-Message-Id: <3>
+Message-ID: <3>
 In-Reply-To: <2>
 References: <0>
 	<1>
@@ -600,22 +600,22 @@ test_expect_success 'thread deep cover-letter' '
 
 cat >expect.deep-cl-irt <<EOF
 ---
-Message-Id: <0>
+Message-ID: <0>
 In-Reply-To: <1>
 References: <1>
 ---
-Message-Id: <2>
+Message-ID: <2>
 In-Reply-To: <0>
 References: <1>
 	<0>
 ---
-Message-Id: <3>
+Message-ID: <3>
 In-Reply-To: <2>
 References: <1>
 	<0>
 	<2>
 ---
-Message-Id: <4>
+Message-ID: <4>
 In-Reply-To: <3>
 References: <1>
 	<0>
diff --git i/t/t4150-am.sh w/t/t4150-am.sh
index 2aaaa0d7de..f41418c6e9 100755
--- i/t/t4150-am.sh
+++ w/t/t4150-am.sh
@@ -103,7 +103,7 @@ test_expect_success setup '
 
 	git format-patch --stdout first >patch1 &&
 	{
-		echo "Message-Id: <1226501681-24923-1-git-send-email-bda@mnsspb.ru>" &&
+		echo "Message-ID: <1226501681-24923-1-git-send-email-bda@mnsspb.ru>" &&
 		echo "X-Fake-Field: Line One" &&
 		echo "X-Fake-Field: Line Two" &&
 		echo "X-Fake-Field: Line Three" &&
@@ -916,7 +916,7 @@ test_expect_success 'am --message-id really adds the message id' '
 	git am --message-id patch1.eml &&
 	test_path_is_missing .git/rebase-apply &&
 	git cat-file commit HEAD | tail -n1 >actual &&
-	grep Message-Id patch1.eml >expected &&
+	grep Message-ID patch1.eml >expected &&
 	test_cmp expected actual
 '
 
@@ -928,7 +928,7 @@ test_expect_success 'am.messageid really adds the message id' '
 	git am patch1.eml &&
 	test_path_is_missing .git/rebase-apply &&
 	git cat-file commit HEAD | tail -n1 >actual &&
-	grep Message-Id patch1.eml >expected &&
+	grep Message-ID patch1.eml >expected &&
 	test_cmp expected actual
 '
 
@@ -939,7 +939,7 @@ test_expect_success 'am --message-id -s signs off after the message id' '
 	git am -s --message-id patch1.eml &&
 	test_path_is_missing .git/rebase-apply &&
 	git cat-file commit HEAD | tail -n2 | head -n1 >actual &&
-	grep Message-Id patch1.eml >expected &&
+	grep Message-ID patch1.eml >expected &&
 	test_cmp expected actual
 '
 
diff --git i/t/t4258/mbox w/t/t4258/mbox
index c62819f3d2..1ae528ba78 100644
--- i/t/t4258/mbox
+++ w/t/t4258/mbox
@@ -2,7 +2,7 @@ From: A U Thor <mail@example.com>
 To: list@example.org
 Subject: [PATCH v2] sample
 Date: Mon,  3 Aug 2020 22:40:55 +0700
-Message-Id: <msg-id@example.com>
+Message-ID: <msg-id@example.com>
 Content-Type: text/plain; charset="utf-8"
 Content-Transfer-Encoding: base64
 
diff --git i/t/t5100/msg0002 w/t/t5100/msg0002
index e2546ec733..1089382425 100644
--- i/t/t5100/msg0002
+++ w/t/t5100/msg0002
@@ -3,7 +3,7 @@ message:
 
 From: Nit Picker <nit.picker@example.net>
 Subject: foo is too old
-Message-Id: <nitpicker.12121212@example.net>
+Message-ID: <nitpicker.12121212@example.net>
 
 Hopefully this would fix the problem stated there.
 
diff --git i/t/t5100/msg0003 w/t/t5100/msg0003
index 1ac68101b1..3402b534a6 100644
--- i/t/t5100/msg0003
+++ w/t/t5100/msg0003
@@ -3,7 +3,7 @@ message:
 
 From: Nit Picker <nit.picker@example.net>
 Subject: foo is too old
-Message-Id: <nitpicker.12121212@example.net>
+Message-ID: <nitpicker.12121212@example.net>
 
 Hopefully this would fix the problem stated there.
 
diff --git i/t/t5100/msg0012--message-id w/t/t5100/msg0012--message-id
index 376e26e9ae..44482958ce 100644
--- i/t/t5100/msg0012--message-id
+++ w/t/t5100/msg0012--message-id
@@ -5,4 +5,4 @@ docutils заменён на python-docutils
 python-docutils. В то время как сам rest2web не нужен.
 
 Signed-off-by: Dmitriy Blinov <bda@mnsspb.ru>
-Message-Id: <1226501681-24923-1-git-send-email-bda@mnsspb.ru>
+Message-ID: <1226501681-24923-1-git-send-email-bda@mnsspb.ru>
diff --git i/t/t5100/quoted-cr.mbox w/t/t5100/quoted-cr.mbox
index 909021bb7a..a529d4de08 100644
--- i/t/t5100/quoted-cr.mbox
+++ w/t/t5100/quoted-cr.mbox
@@ -3,7 +3,7 @@ From: A U Thor <mail@example.com>
 To: list@example.org
 Subject: [PATCH v2] sample
 Date: Mon,  3 Aug 2020 22:40:55 +0700
-Message-Id: <msg-id@example.com>
+Message-ID: <msg-id@example.com>
 Content-Type: text/plain; charset="utf-8"
 Content-Transfer-Encoding: base64
 
@@ -27,7 +27,7 @@ From: A U Thor <mail@example.com>
 To: list@example.org
 Subject: [PATCH v2] sample
 Date: Mon,  3 Aug 2020 22:40:55 +0700
-Message-Id: <msg-id2@example.com>
+Message-ID: <msg-id2@example.com>
 Content-Type: text/plain; charset="utf-8"
 Content-Transfer-Encoding: base64
 
diff --git i/t/t5100/sample.mbox w/t/t5100/sample.mbox
index 6d4d0e4474..4a54ee5171 100644
--- i/t/t5100/sample.mbox
+++ w/t/t5100/sample.mbox
@@ -35,7 +35,7 @@ message:
 
 From: Nit Picker <nit.picker@example.net>
 Subject: foo is too old
-Message-Id: <nitpicker.12121212@example.net>
+Message-ID: <nitpicker.12121212@example.net>
 
 Hopefully this would fix the problem stated there.
 
@@ -78,7 +78,7 @@ message:
 
 From: Nit Picker <nit.picker@example.net>
 Subject: foo is too old
-Message-Id: <nitpicker.12121212@example.net>
+Message-ID: <nitpicker.12121212@example.net>
 
 Hopefully this would fix the problem stated there.
 
@@ -508,7 +508,7 @@ From bda@mnsspb.ru Wed Nov 12 17:54:41 2008
 From: Dmitriy Blinov <bda@mnsspb.ru>
 To: navy-patches@dinar.mns.mnsspb.ru
 Date: Wed, 12 Nov 2008 17:54:41 +0300
-Message-Id: <1226501681-24923-1-git-send-email-bda@mnsspb.ru>
+Message-ID: <1226501681-24923-1-git-send-email-bda@mnsspb.ru>
 X-Mailer: git-send-email 1.5.6.5
 MIME-Version: 1.0
 Content-Type: text/plain;
diff --git i/t/t9001-send-email.sh w/t/t9001-send-email.sh
index aa0c20499b..ce09cf1fe3 100755
--- i/t/t9001-send-email.sh
+++ w/t/t9001-send-email.sh
@@ -11,7 +11,7 @@ PREREQ="PERL"
 
 replace_variable_fields () {
 	sed	-e "s/^\(Date:\).*/\1 DATE-STRING/" \
-		-e "s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/" \
+		-e "s/^\(Message-ID:\).*/\1 MESSAGE-ID-STRING/" \
 		-e "s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/"
 }
 
@@ -224,7 +224,7 @@ Cc: cc@example.com,
 	two@example.com
 Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
-Message-Id: MESSAGE-ID-STRING
+Message-ID: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
 In-Reply-To: <unique-message-id@example.com>
 References: <unique-message-id@example.com>
@@ -616,7 +616,7 @@ test_expect_success $PREREQ 'In-Reply-To without --chain-reply-to' '
 	sed -n -e "s/^In-Reply-To: *\(.*\)/\1/p" msgtxt1 >actual &&
 	test_cmp expect actual &&
 	# Second and subsequent messages are replies to the first one
-	sed -n -e "s/^Message-Id: *\(.*\)/\1/p" msgtxt1 >expect &&
+	sed -n -e "s/^Message-ID: *\(.*\)/\1/p" msgtxt1 >expect &&
 	sed -n -e "s/^In-Reply-To: *\(.*\)/\1/p" msgtxt2 >actual &&
 	test_cmp expect actual &&
 	sed -n -e "s/^In-Reply-To: *\(.*\)/\1/p" msgtxt3 >actual &&
@@ -636,10 +636,10 @@ test_expect_success $PREREQ 'In-Reply-To with --chain-reply-to' '
 		2>errors &&
 	sed -n -e "s/^In-Reply-To: *\(.*\)/\1/p" msgtxt1 >actual &&
 	test_cmp expect actual &&
-	sed -n -e "s/^Message-Id: *\(.*\)/\1/p" msgtxt1 >expect &&
+	sed -n -e "s/^Message-ID: *\(.*\)/\1/p" msgtxt1 >expect &&
 	sed -n -e "s/^In-Reply-To: *\(.*\)/\1/p" msgtxt2 >actual &&
 	test_cmp expect actual &&
-	sed -n -e "s/^Message-Id: *\(.*\)/\1/p" msgtxt2 >expect &&
+	sed -n -e "s/^Message-ID: *\(.*\)/\1/p" msgtxt2 >expect &&
 	sed -n -e "s/^In-Reply-To: *\(.*\)/\1/p" msgtxt3 >actual &&
 	test_cmp expect actual
 '
@@ -712,7 +712,7 @@ Cc: cc@example.com,
 	two@example.com
 Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
-Message-Id: MESSAGE-ID-STRING
+Message-ID: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
 MIME-Version: 1.0
 Content-Transfer-Encoding: 8bit
@@ -758,7 +758,7 @@ Cc: A <author@example.com>,
 	two@example.com
 Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
-Message-Id: MESSAGE-ID-STRING
+Message-ID: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
 MIME-Version: 1.0
 Content-Transfer-Encoding: 8bit
@@ -795,7 +795,7 @@ Cc: A <author@example.com>,
 	C O Mitter <committer@example.com>
 Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
-Message-Id: MESSAGE-ID-STRING
+Message-ID: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
 MIME-Version: 1.0
 Content-Transfer-Encoding: 8bit
@@ -823,7 +823,7 @@ From: Example <from@example.com>
 To: to@example.com
 Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
-Message-Id: MESSAGE-ID-STRING
+Message-ID: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
 MIME-Version: 1.0
 Content-Transfer-Encoding: 8bit
@@ -859,7 +859,7 @@ Cc: A <author@example.com>,
 	cc-cmd@example.com
 Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
-Message-Id: MESSAGE-ID-STRING
+Message-ID: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
 MIME-Version: 1.0
 Content-Transfer-Encoding: 8bit
@@ -892,7 +892,7 @@ Cc: A <author@example.com>,
 	two@example.com
 Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
-Message-Id: MESSAGE-ID-STRING
+Message-ID: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
 MIME-Version: 1.0
 Content-Transfer-Encoding: 8bit
@@ -925,7 +925,7 @@ Cc: A <author@example.com>,
 	two@example.com
 Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
-Message-Id: MESSAGE-ID-STRING
+Message-ID: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
 MIME-Version: 1.0
 Content-Transfer-Encoding: 8bit
@@ -962,7 +962,7 @@ Cc: A <author@example.com>,
 	C O Mitter <committer@example.com>
 Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
-Message-Id: MESSAGE-ID-STRING
+Message-ID: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
 MIME-Version: 1.0
 Content-Transfer-Encoding: 8bit
@@ -992,7 +992,7 @@ Cc: A <author@example.com>,
 	C O Mitter <committer@example.com>
 Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
-Message-Id: MESSAGE-ID-STRING
+Message-ID: MESSAGE-ID-STRING
 X-Mailer: X-MAILER-STRING
 MIME-Version: 1.0
 Content-Transfer-Encoding: 8bit
@@ -1477,7 +1477,7 @@ test_expect_success $PREREQ 'To headers from files reset each patch' '
 test_expect_success $PREREQ 'setup expect' '
 cat >email-using-8bit <<\EOF
 From fe6ecc66ece37198fe5db91fa2fc41d9f4fe5cc4 Mon Sep 17 00:00:00 2001
-Message-Id: <bogus-message-id@example.com>
+Message-ID: <bogus-message-id@example.com>
 From: author@example.com
 Date: Sat, 12 Jun 2010 15:53:58 +0200
 Subject: subject goes here
@@ -1563,7 +1563,7 @@ test_expect_success $PREREQ '--8bit-encoding overrides sendemail.8bitEncoding' '
 test_expect_success $PREREQ 'setup expect' '
 	cat >email-using-8bit <<-\EOF
 	From fe6ecc66ece37198fe5db91fa2fc41d9f4fe5cc4 Mon Sep 17 00:00:00 2001
-	Message-Id: <bogus-message-id@example.com>
+	Message-ID: <bogus-message-id@example.com>
 	From: author@example.com
 	Date: Sat, 12 Jun 2010 15:53:58 +0200
 	Subject: Dieser Betreff enthält auch einen Umlaut!
@@ -1592,7 +1592,7 @@ test_expect_success $PREREQ '--8bit-encoding also treats subject' '
 test_expect_success $PREREQ 'setup expect' '
 	cat >email-using-8bit <<-\EOF
 	From fe6ecc66ece37198fe5db91fa2fc41d9f4fe5cc4 Mon Sep 17 00:00:00 2001
-	Message-Id: <bogus-message-id@example.com>
+	Message-ID: <bogus-message-id@example.com>
 	From: A U Thor <author@example.com>
 	Date: Sat, 12 Jun 2010 15:53:58 +0200
 	Content-Type: text/plain; charset=UTF-8
@@ -1673,7 +1673,7 @@ test_expect_success $PREREQ '8-bit and sendemail.transferencoding=base64' '
 test_expect_success $PREREQ 'setup expect' '
 	cat >email-using-qp <<-\EOF
 	From fe6ecc66ece37198fe5db91fa2fc41d9f4fe5cc4 Mon Sep 17 00:00:00 2001
-	Message-Id: <bogus-message-id@example.com>
+	Message-ID: <bogus-message-id@example.com>
 	From: A U Thor <author@example.com>
 	Date: Sat, 12 Jun 2010 15:53:58 +0200
 	MIME-Version: 1.0
@@ -1699,7 +1699,7 @@ test_expect_success $PREREQ 'convert from quoted-printable to base64' '
 test_expect_success $PREREQ 'setup expect' "
 tr -d '\\015' | tr '%' '\\015' >email-using-crlf <<EOF
 From fe6ecc66ece37198fe5db91fa2fc41d9f4fe5cc4 Mon Sep 17 00:00:00 2001
-Message-Id: <bogus-message-id@example.com>
+Message-ID: <bogus-message-id@example.com>
 From: A U Thor <author@example.com>
 Date: Sat, 12 Jun 2010 15:53:58 +0200
 Content-Type: text/plain; charset=UTF-8

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-23 18:52   ` Eric Wong
  2021-11-24  1:37     ` Junio C Hamano
@ 2021-11-24  2:08     ` Ævar Arnfjörð Bjarmason
  2021-11-26  7:29     ` Thorsten Leemhuis
  2 siblings, 0 replies; 32+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-24  2:08 UTC (permalink / raw)
  To: Eric Wong
  Cc: Thorsten Leemhuis, workflows, linux-kernel, Konstantin Ryabitsev,
	Jonathan Corbet, git


On Tue, Nov 23 2021, Eric Wong wrote:

> Thorsten Leemhuis <linux@leemhuis.info> wrote:
>> diff --git a/Documentation/maintainer/configure-git.rst b/Documentation/maintainer/configure-git.rst
>> index 80ae5030a590..8429d45d661c 100644
>> --- a/Documentation/maintainer/configure-git.rst
>> +++ b/Documentation/maintainer/configure-git.rst
>
> <snip>, +cc git@vger
>
>> @@ -56,7 +56,7 @@ by adding the following hook into your git:
>>  	$ cat >.git/hooks/applypatch-msg <<'EOF'
>>  	#!/bin/sh
>>  	. git-sh-setup
>> -	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|g;' "$1"
>> +	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Reviewed: https://lore.kernel.org/r/$1|g;' "$1"
>
> Side note: that regexp should match "Message-ID" case-insensitively.
> git send-email is an outlier in its capitalization of "Message-Id",
> most RFCs capitalize it "Message-ID", as do common MUAs.
>
> git send-email's capitalization does annoy me and I've looked
> into changing it; but there's a bunch of tests and probably
> dependent code that also need to be updated...

"git format-patch" does it without send-email, but I see that send-email
will then parse its output, and would turn any capitalization into the
"Message-Id" form again.

We could probably just have it preserve whatever capitalization it finds
if there's an existing header, we wouldn't fix anything, but we'd move
the blame around a bit :)

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-24  1:37     ` Junio C Hamano
@ 2021-11-24  6:12       ` Eric Wong
  2021-11-26 12:49       ` Ævar Arnfjörð Bjarmason
  1 sibling, 0 replies; 32+ messages in thread
From: Eric Wong @ 2021-11-24  6:12 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Thorsten Leemhuis, workflows, linux-kernel, Konstantin Ryabitsev,
	Jonathan Corbet, git

Junio C Hamano <gitster@pobox.com> wrote:
> Eric Wong <e@80x24.org> writes:
> 
> > git send-email's capitalization does annoy me and I've looked
> > into changing it; but there's a bunch of tests and probably
> > dependent code that also need to be updated...
> 
> It does annoy me, too, and I do not mind if it gets "fixed", but I
> do not know if the cost for vetting a bulk update like that is worth
> it.  There is another one outside send-email in log-tree.c that is
> responsible for output from format-patch.

I support updating the documentation in git, first; then perhaps
making tests case-insensitive (which could be a complex
change...).

Unfortunately, changing the send-email + log-tree.c code would
break existing users of the applypatch-msg hook sample in
linux/Documentation/maintainer/configure-git.rst which has
been case-sensitive for around for 2 years:
https://lore.kernel.org/r/20191118223019.81708-1-linus.walleij@linaro.org

:<

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-23 18:52   ` Eric Wong
  2021-11-24  1:37     ` Junio C Hamano
  2021-11-24  2:08     ` Ævar Arnfjörð Bjarmason
@ 2021-11-26  7:29     ` Thorsten Leemhuis
  2021-11-26 17:11       ` Eric Wong
  2 siblings, 1 reply; 32+ messages in thread
From: Thorsten Leemhuis @ 2021-11-26  7:29 UTC (permalink / raw)
  To: Eric Wong
  Cc: workflows, linux-kernel, Konstantin Ryabitsev, Jonathan Corbet,
	git, Linus Walleij, Kees Cook

Ccing Linus Walleij, who added this, and Kees, who apparently came up
with this originally.

On 23.11.21 19:52, Eric Wong wrote:
> Thorsten Leemhuis <linux@leemhuis.info> wrote:
>> diff --git a/Documentation/maintainer/configure-git.rst b/Documentation/maintainer/configure-git.rst
>> index 80ae5030a590..8429d45d661c 100644
>> --- a/Documentation/maintainer/configure-git.rst
>> +++ b/Documentation/maintainer/configure-git.rst
> 
> <snip>, +cc git@vger
> 
>> @@ -56,7 +56,7 @@ by adding the following hook into your git:
>>  	$ cat >.git/hooks/applypatch-msg <<'EOF'
>>  	#!/bin/sh
>>  	. git-sh-setup
>> -	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|g;' "$1"
>> +	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Reviewed: https://lore.kernel.org/r/$1|g;' "$1"
> 
> Side note: that regexp should match "Message-ID" case-insensitively.
> git send-email is an outlier in its capitalization of "Message-Id",
> most RFCs capitalize it "Message-ID", as do common MUAs.

Argh :-/

It's still totally unclear if that or a similar patch will be accepted.
And even if it is: the "don't do two different things in one commit"
rule might not be that strict enforced when it comes to the Linux
kernel's docs, but changing this regexp as part of another patch crosses
the line.

IOW: we afaics need a separate patch to make the regexp
case-insensitively. Eric, do you want to submit one, as you brought it
up? Or are there any other volunteers?

Ciao, Thorsten

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-24  1:37     ` Junio C Hamano
  2021-11-24  6:12       ` Eric Wong
@ 2021-11-26 12:49       ` Ævar Arnfjörð Bjarmason
  1 sibling, 0 replies; 32+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-26 12:49 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Eric Wong, Thorsten Leemhuis, workflows, linux-kernel,
	Konstantin Ryabitsev, Jonathan Corbet, git


On Tue, Nov 23 2021, Junio C Hamano wrote:

> Eric Wong <e@80x24.org> writes:
>
>> git send-email's capitalization does annoy me and I've looked
>> into changing it; but there's a bunch of tests and probably
>> dependent code that also need to be updated...
> [...]
> diff --git i/git-send-email.perl w/git-send-email.perl
> index 5262d88ee3..a61134c7d3 100755
> --- i/git-send-email.perl
> +++ w/git-send-email.perl
> @@ -1494,7 +1494,7 @@ sub send_message {
>  To: $to${ccline}
>  Subject: $subject
>  Date: $date
> -Message-Id: $message_id
> +Message-ID: $message_id
>  ";
>  	if ($use_xmailer) {
>  		$header .= "X-Mailer: git-send-email $gitversion\n";

Perhaps one way to split this & make it more readable is to split this,
i.e. the mesage-id's send-email itself generates & tests, usually it
passes along format-patch's.

> @@ -1789,7 +1789,7 @@ sub process_file {
>  				$has_mime_version = 1;
>  				push @xh, $_;
>  			}
> -			elsif (/^Message-Id: (.*)/i) {
> +			elsif (/^Message-ID: (.*)/i) {
>  				$message_id = $1;
>  			}
>  			elsif (/^Content-Transfer-Encoding: (.*)/i) {

Not strictly needed due to the /i, maybe splitting out cosmetic changes
would be better?

I also notice we have various hits for "git grep message-id", including
regex checks you didn't update here.

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-26  7:29     ` Thorsten Leemhuis
@ 2021-11-26 17:11       ` Eric Wong
  2021-11-27 19:32         ` Thorsten Leemhuis
  0 siblings, 1 reply; 32+ messages in thread
From: Eric Wong @ 2021-11-26 17:11 UTC (permalink / raw)
  To: Thorsten Leemhuis
  Cc: workflows, linux-kernel, Konstantin Ryabitsev, Jonathan Corbet,
	git, Linus Walleij, Kees Cook

Thorsten Leemhuis <linux@leemhuis.info> wrote:
> Ccing Linus Walleij, who added this, and Kees, who apparently came up
> with this originally.
> 
> On 23.11.21 19:52, Eric Wong wrote:
> > Thorsten Leemhuis <linux@leemhuis.info> wrote:
> >> diff --git a/Documentation/maintainer/configure-git.rst b/Documentation/maintainer/configure-git.rst
> >> index 80ae5030a590..8429d45d661c 100644
> >> --- a/Documentation/maintainer/configure-git.rst
> >> +++ b/Documentation/maintainer/configure-git.rst
> > 
> > <snip>, +cc git@vger
> > 
> >> @@ -56,7 +56,7 @@ by adding the following hook into your git:
> >>  	$ cat >.git/hooks/applypatch-msg <<'EOF'
> >>  	#!/bin/sh
> >>  	. git-sh-setup
> >> -	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|g;' "$1"
> >> +	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Reviewed: https://lore.kernel.org/r/$1|g;' "$1"
> > 
> > Side note: that regexp should match "Message-ID" case-insensitively.
> > git send-email is an outlier in its capitalization of "Message-Id",
> > most RFCs capitalize it "Message-ID", as do common MUAs.
> 
> Argh :-/
> 
> It's still totally unclear if that or a similar patch will be accepted.
> And even if it is: the "don't do two different things in one commit"
> rule might not be that strict enforced when it comes to the Linux
> kernel's docs, but changing this regexp as part of another patch crosses
> the line.
> 
> IOW: we afaics need a separate patch to make the regexp
> case-insensitively. Eric, do you want to submit one, as you brought it
> up? Or are there any other volunteers?

I suggest you turn this into a 2 patch series to avoid conflicts
for a trivial change.  I don't even have a kernel worktree handy
at the moment (ENOSPC :x)

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-26 17:11       ` Eric Wong
@ 2021-11-27 19:32         ` Thorsten Leemhuis
  2021-11-27 19:52           ` Eric Wong
  0 siblings, 1 reply; 32+ messages in thread
From: Thorsten Leemhuis @ 2021-11-27 19:32 UTC (permalink / raw)
  To: Eric Wong
  Cc: workflows, linux-kernel, Konstantin Ryabitsev, Jonathan Corbet,
	git, Linus Walleij, Kees Cook

On 26.11.21 18:11, Eric Wong wrote:
> Thorsten Leemhuis <linux@leemhuis.info> wrote:
>> Ccing Linus Walleij, who added this, and Kees, who apparently came up
>> with this originally.
>>
>> On 23.11.21 19:52, Eric Wong wrote:
>>> Thorsten Leemhuis <linux@leemhuis.info> wrote:
>>>> diff --git a/Documentation/maintainer/configure-git.rst b/Documentation/maintainer/configure-git.rst
>>>> index 80ae5030a590..8429d45d661c 100644
>>>> --- a/Documentation/maintainer/configure-git.rst
>>>> +++ b/Documentation/maintainer/configure-git.rst
>>>
>>> <snip>, +cc git@vger
>>>
>>>> @@ -56,7 +56,7 @@ by adding the following hook into your git:
>>>>  	$ cat >.git/hooks/applypatch-msg <<'EOF'
>>>>  	#!/bin/sh
>>>>  	. git-sh-setup
>>>> -	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|g;' "$1"
>>>> +	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Reviewed: https://lore.kernel.org/r/$1|g;' "$1"
>>>
>>> Side note: that regexp should match "Message-ID" case-insensitively.
>>> git send-email is an outlier in its capitalization of "Message-Id",
>>> most RFCs capitalize it "Message-ID", as do common MUAs.
>>
>> Argh :-/
>>
>> It's still totally unclear if that or a similar patch will be accepted.
>> And even if it is: the "don't do two different things in one commit"
>> rule might not be that strict enforced when it comes to the Linux
>> kernel's docs, but changing this regexp as part of another patch crosses
>> the line.
>>
>> IOW: we afaics need a separate patch to make the regexp
>> case-insensitively. Eric, do you want to submit one, as you brought it
>> up? Or are there any other volunteers?
> 
> I suggest you turn this into a 2 patch series to avoid conflicts
> for a trivial change.  I don't even have a kernel worktree handy
> at the moment (ENOSPC :x)

:-D

Will do this in a couple of days, unless Linus or Kees speak up.

Just to be sure I'll do what you expect to be done: I assume you want to see
it changed like this?

-	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|g;' "$1"
+	perl -pi -e 's|^Message-I[dD]:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|g;' "$1"

Or are there even more variants of Message-ID out there known that
need to be taken into account?

Ciao, Thorsten

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-27 19:32         ` Thorsten Leemhuis
@ 2021-11-27 19:52           ` Eric Wong
  2021-11-27 20:20             ` Junio C Hamano
  0 siblings, 1 reply; 32+ messages in thread
From: Eric Wong @ 2021-11-27 19:52 UTC (permalink / raw)
  To: Thorsten Leemhuis
  Cc: workflows, linux-kernel, Konstantin Ryabitsev, Jonathan Corbet,
	git, Linus Walleij, Kees Cook

Thorsten Leemhuis <linux@leemhuis.info> wrote:
> Just to be sure I'll do what you expect to be done: I assume you want to see
> it changed like this?
> 
> -	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|g;' "$1"
> +	perl -pi -e 's|^Message-I[dD]:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|g;' "$1"
> 
> Or are there even more variants of Message-ID out there known that
> need to be taken into account?

The entire match should be case-insensitive[1], so I'd add `i'
at the end:

	perl -pi -e 's|^Message-ID:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|gi;' "$1"

Fwiw, every mail and HTTP/1.x header parser I've looked at works
case-insensitively.  Also, I'm not sure if `g' is needed, actually...

[1] https://datatracker.ietf.org/doc/html/rfc822#section-3.4.7

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-27 19:52           ` Eric Wong
@ 2021-11-27 20:20             ` Junio C Hamano
  2021-11-29 12:03               ` Jani Nikula
  0 siblings, 1 reply; 32+ messages in thread
From: Junio C Hamano @ 2021-11-27 20:20 UTC (permalink / raw)
  To: Eric Wong
  Cc: Thorsten Leemhuis, workflows, linux-kernel, Konstantin Ryabitsev,
	Jonathan Corbet, git, Linus Walleij, Kees Cook

Eric Wong <e@80x24.org> writes:

> Thorsten Leemhuis <linux@leemhuis.info> wrote:
>> Just to be sure I'll do what you expect to be done: I assume you want to see
>> it changed like this?
>> 
>> -	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|g;' "$1"
> ...
> The entire match should be case-insensitive[1], so I'd add `i'
> at the end:
>
> 	perl -pi -e 's|^Message-ID:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|gi;' "$1"
>
> Fwiw, every mail and HTTP/1.x header parser I've looked at works
> case-insensitively.  Also, I'm not sure if `g' is needed, actually...

It is left anchored with "^" so it would be hard to match more than
once on the same line ;-)

I agree that it is the right solution to make the whole thing
case-insensitive by adding 'i' at the end.

FWIW, the RFC first says this:

    1.2.2. Syntactic notation

       This standard uses the Augmented Backus-Naur Form (ABNF) notation
       specified in [RFC2234] for the formal definitions of the syntax of
       messages.  Characters will be specified either by a decimal value
       (e.g., the value %d65 for uppercase A and %d97 for lowercase A) or by
       a case-insensitive literal value enclosed in quotation marks (e.g.,
       "A" for either uppercase or lowercase A).

and then goes on to define how message-id should look like.

    3.6.4. Identification fields

    message-id      =       "Message-ID:" msg-id CRLF


But if you go the "add /i at the end" route, you do not have to
upcase "d" to "D" and that may reduce the patch noise (it only
matters if the patch viewer highlights letter-by-letter changes for
your recipients).

HTH

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-27 20:20             ` Junio C Hamano
@ 2021-11-29 12:03               ` Jani Nikula
  2021-11-29 17:10                 ` Steven Rostedt
                                   ` (2 more replies)
  0 siblings, 3 replies; 32+ messages in thread
From: Jani Nikula @ 2021-11-29 12:03 UTC (permalink / raw)
  To: Junio C Hamano, Eric Wong
  Cc: Thorsten Leemhuis, workflows, linux-kernel, Konstantin Ryabitsev,
	Jonathan Corbet, git, Linus Walleij, Kees Cook

On Sat, 27 Nov 2021, Junio C Hamano <gitster@pobox.com> wrote:
> Eric Wong <e@80x24.org> writes:
>
>> Thorsten Leemhuis <linux@leemhuis.info> wrote:
>>> Just to be sure I'll do what you expect to be done: I assume you want to see
>>> it changed like this?
>>> 
>>> -	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|g;' "$1"
>> ...
>> The entire match should be case-insensitive[1], so I'd add `i'
>> at the end:
>>
>> 	perl -pi -e 's|^Message-ID:\s*<?([^>]+)>?$|Link: https://lore.kernel.org/r/$1|gi;' "$1"
>>
>> Fwiw, every mail and HTTP/1.x header parser I've looked at works
>> case-insensitively.  Also, I'm not sure if `g' is needed, actually...
>
> It is left anchored with "^" so it would be hard to match more than
> once on the same line ;-)
>
> I agree that it is the right solution to make the whole thing
> case-insensitive by adding 'i' at the end.
>
> FWIW, the RFC first says this:
>
>     1.2.2. Syntactic notation
>
>        This standard uses the Augmented Backus-Naur Form (ABNF) notation
>        specified in [RFC2234] for the formal definitions of the syntax of
>        messages.  Characters will be specified either by a decimal value
>        (e.g., the value %d65 for uppercase A and %d97 for lowercase A) or by
>        a case-insensitive literal value enclosed in quotation marks (e.g.,
>        "A" for either uppercase or lowercase A).
>
> and then goes on to define how message-id should look like.
>
>     3.6.4. Identification fields
>
>     message-id      =       "Message-ID:" msg-id CRLF
>
>
> But if you go the "add /i at the end" route, you do not have to
> upcase "d" to "D" and that may reduce the patch noise (it only
> matters if the patch viewer highlights letter-by-letter changes for
> your recipients).

From the RFC nitpicking department, msg-id is allowed to contain CFWS
(comments and folding white space) outside the angle brackets, which
means you could have RFC compliant Message-ID header field:

Message-ID: 
  <message-id@example.com>

or

Message-ID: (comment) 
  <message-id@example.com>

or even worse, really.

The moral of the story is that you should always offload the header
parsing to some tool or library designed to do that.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-29 12:03               ` Jani Nikula
@ 2021-11-29 17:10                 ` Steven Rostedt
  2021-11-29 17:18                 ` Junio C Hamano
  2021-11-29 17:26                 ` Eric Wong
  2 siblings, 0 replies; 32+ messages in thread
From: Steven Rostedt @ 2021-11-29 17:10 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Junio C Hamano, Eric Wong, Thorsten Leemhuis, workflows,
	linux-kernel, Konstantin Ryabitsev, Jonathan Corbet, git,
	Linus Walleij, Kees Cook

On Mon, 29 Nov 2021 14:03:09 +0200
Jani Nikula <jani.nikula@intel.com> wrote:

> >From the RFC nitpicking department, msg-id is allowed to contain CFWS  
> (comments and folding white space) outside the angle brackets, which
> means you could have RFC compliant Message-ID header field:
> 
> Message-ID: 
>   <message-id@example.com>

My scripts have already been hit by this. (I've been lazy and not fixed it,
but instead, just edit the file that it is parsing manually, to be on one
line :-p)

-- Steve

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-29 12:03               ` Jani Nikula
  2021-11-29 17:10                 ` Steven Rostedt
@ 2021-11-29 17:18                 ` Junio C Hamano
  2021-11-29 19:18                   ` Jani Nikula
  2021-11-29 17:26                 ` Eric Wong
  2 siblings, 1 reply; 32+ messages in thread
From: Junio C Hamano @ 2021-11-29 17:18 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Eric Wong, Thorsten Leemhuis, workflows, linux-kernel,
	Konstantin Ryabitsev, Jonathan Corbet, git, Linus Walleij,
	Kees Cook

Jani Nikula <jani.nikula@intel.com> writes:

> From the RFC nitpicking department, ...
>
> Message-ID: (comment) 
>   <message-id@example.com>

Thanks for a fun piece; the (comment) is quite interesting.

I wasn't having fun with RFC nitpicking, though.  I was reacting to
this part of the message I was responding to ...

>>> Fwiw, every mail and HTTP/1.x header parser I've looked at works
>>> case-insensitively.  Also, I'm not sure if `g' is needed, actually...

... to say that "works case-insensitively" may not just be
empirically correct, but RFC backs him up, to Eric.

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-29 12:03               ` Jani Nikula
  2021-11-29 17:10                 ` Steven Rostedt
  2021-11-29 17:18                 ` Junio C Hamano
@ 2021-11-29 17:26                 ` Eric Wong
  2021-11-29 19:20                   ` Jani Nikula
  2021-11-30  8:24                   ` Geert Uytterhoeven
  2 siblings, 2 replies; 32+ messages in thread
From: Eric Wong @ 2021-11-29 17:26 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Junio C Hamano, Thorsten Leemhuis, workflows, linux-kernel,
	Konstantin Ryabitsev, Jonathan Corbet, git, Linus Walleij,
	Kees Cook

Jani Nikula <jani.nikula@intel.com> wrote:
> From the RFC nitpicking department, msg-id is allowed to contain CFWS
> (comments and folding white space) outside the angle brackets, which
> means you could have RFC compliant Message-ID header field:
> 
> Message-ID: 
>   <message-id@example.com>
> 
> or
> 
> Message-ID: (comment) 
>   <message-id@example.com>
> 
> or even worse, really.
> 
> The moral of the story is that you should always offload the header
> parsing to some tool or library designed to do that.

It's a bit much for common cases with git-send-email and
reasonable MUAs, I think.  I don't know if formail is commonly
installed, nowadays...

Fwiw, the code running lore uses something like this:

	/^Message-ID:[ \t]*([^\n]*\r?\n # 1st line
			# continuation lines:
			(?:[^:\n]*?[ \t]+[^\n]*\r?\n)*)
			/ismx

I'm fine with this non-trivial regexp being included with
GPL-2.0 code; but it could be too big for a one-liner *shrug*

... And <([^>]+)>/s to extract Message-IDs, but ISTR the code
behind lore doesn't handle spaces inside <> properly, but I'm
not sure if there's enough valid, non-spam messages with them...

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-29 17:18                 ` Junio C Hamano
@ 2021-11-29 19:18                   ` Jani Nikula
  0 siblings, 0 replies; 32+ messages in thread
From: Jani Nikula @ 2021-11-29 19:18 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Eric Wong, Thorsten Leemhuis, workflows, linux-kernel,
	Konstantin Ryabitsev, Jonathan Corbet, git, Linus Walleij,
	Kees Cook

On Mon, 29 Nov 2021, Junio C Hamano <gitster@pobox.com> wrote:
> I wasn't having fun with RFC nitpicking, though.

I didn't mean to imply you were, I was saying I was!

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-29 17:26                 ` Eric Wong
@ 2021-11-29 19:20                   ` Jani Nikula
  2021-11-30  8:24                   ` Geert Uytterhoeven
  1 sibling, 0 replies; 32+ messages in thread
From: Jani Nikula @ 2021-11-29 19:20 UTC (permalink / raw)
  To: Eric Wong
  Cc: Junio C Hamano, Thorsten Leemhuis, workflows, linux-kernel,
	Konstantin Ryabitsev, Jonathan Corbet, git, Linus Walleij,
	Kees Cook

On Mon, 29 Nov 2021, Eric Wong <e@80x24.org> wrote:
> Jani Nikula <jani.nikula@intel.com> wrote:
>> The moral of the story is that you should always offload the header
>> parsing to some tool or library designed to do that.
>
> It's a bit much for common cases with git-send-email and
> reasonable MUAs, I think.

I think you can have unreasonable MDAs in between, though!

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-22  7:33 ` [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:' Thorsten Leemhuis
  2021-11-22 16:29   ` Steven Rostedt
  2021-11-23 18:52   ` Eric Wong
@ 2021-11-29 22:16   ` Jonathan Corbet
  2021-11-30 13:10     ` Thorsten Leemhuis
  2 siblings, 1 reply; 32+ messages in thread
From: Jonathan Corbet @ 2021-11-29 22:16 UTC (permalink / raw)
  To: Thorsten Leemhuis, workflows
  Cc: Linux Kernel Mailing List, Konstantin Ryabitsev

Thorsten Leemhuis <linux@leemhuis.info> writes:

> Introduce the tags 'Reported:' and 'Reviewed:' in addition to 'Link:',
> as the latter is overloaded and hence doesn't indicate what the provided
> URL is about. Documenting these also provides clarity, as a few
> developers have used 'References:' to point to problem reports;
> nevertheless 'Reported:' was chosen for this purpose, as it perfectly
> matches up with the 'Reported-by:' tag commonly used already and needed
> in this situation already.
>
> Signed-off-by: Thorsten Leemhuis <linux@leemhuis.info>
> To: workflows@vger.kernel.org

Thanks for flooding my inbox during a holiday week :)  Just looking at
this now.

> v1/RFC:
> - first, *rough version* to see how this idea is received in the
>   community
> ---
>  Documentation/maintainer/configure-git.rst   |  6 +--
>  Documentation/process/5.Posting.rst          | 54 ++++++++++++++------
>  Documentation/process/submitting-patches.rst | 22 ++++----
>  3 files changed, 53 insertions(+), 29 deletions(-)
>
> diff --git a/Documentation/maintainer/configure-git.rst b/Documentation/maintainer/configure-git.rst
> index 80ae5030a590..8429d45d661c 100644
> --- a/Documentation/maintainer/configure-git.rst
> +++ b/Documentation/maintainer/configure-git.rst
> @@ -40,12 +40,12 @@ Creating commit links to lore.kernel.org
>  The web site http://lore.kernel.org is meant as a grand archive of all mail
>  list traffic concerning or influencing the kernel development. Storing archives
>  of patches here is a recommended practice, and when a maintainer applies a
> -patch to a subsystem tree, it is a good idea to provide a Link: tag with a
> +patch to a subsystem tree, it is a good idea to provide a Reviewed: tag with a
>  reference back to the lore archive so that people that browse the commit
>  history can find related discussions and rationale behind a certain change.
>  The link tag will look like this:
>  
> -    Link: https://lore.kernel.org/r/<message-id>
> +    Reviewed: https://lore.kernel.org/r/<message-id>

The *link* tag will look like that?

[...]

> +The tags in common use are:
> +
> + - ``Reported:`` points to a report of a problem fixed by this patch. The
> +   provided URL thus might point to a entry in a bug tracker or a mail in a
> +   mailing list archive. Typically this tag is followed by a "Reported-by:"
> +   tag (see below).
> +
> + - ``Link:`` points to websites providing additional backgrounds or details,
> +   for example a document with a specification implemented by the patch.

So this is a serious change from how Link: is used now, and runs counter
to the scripts used by a lot of maintainers.  I suspect that this thread
is only as short as it is because a lot of people haven't seen this yet;
it could be a hard change to sell.

Also, I think that documents like specs should be called out separately
in the changelog, with text saying what they actually are.

> + - ``Reviewed:`` ignore this, as maintainers add it when applying a patch, to
> +   make the commit point to the latest public review of the patch.

Another question would be: what's the interplay between the (quite
similar) "Reviewed" and "Reviewed-by" tags (and the same for the report
tags).  If there's a "Reviewed" do we still need "Reviewed-by"?  That
should be spelled out, whichever way is wanted.

I do worry that the similarity is going to lead to a certain amount of
confusion and use of the wrong tag.  People have a hard time getting all
the tags we have now right; adding more that look almost like the
existing ones seems like a recipe for trouble.

For these reasons, I would be more inclined toward Konstantin's
suggestion of adding notes to the existing Link: tags.

> +A third kind of tags are used to document which developers were involved in
> +the development of the patch. Each of these uses this format::
>  
>  	tag: Full Name <email address>  optional-other-stuff
>  
>  The tags in common use are:
>  
> - - Signed-off-by: this is a developer's certification that he or she has
> + - ``Signed-off-by:`` is a developer's certification that he or she has

So this markup addition is a separate change that would belong in its
own patch.  Do we really need it, though?  It clutters the text and
irritates the anti-RST minority (which has been mercifully quiet
recently) without really adding any benefit.

Thanks,

jon

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-29 17:26                 ` Eric Wong
  2021-11-29 19:20                   ` Jani Nikula
@ 2021-11-30  8:24                   ` Geert Uytterhoeven
  2021-12-08 13:41                     ` Thorsten Leemhuis
  1 sibling, 1 reply; 32+ messages in thread
From: Geert Uytterhoeven @ 2021-11-30  8:24 UTC (permalink / raw)
  To: Eric Wong
  Cc: Jani Nikula, Junio C Hamano, Thorsten Leemhuis, workflows,
	Linux Kernel Mailing List, Konstantin Ryabitsev, Jonathan Corbet,
	Git Mailing List, Linus Walleij, Kees Cook

Hi Eric,

On Mon, Nov 29, 2021 at 11:29 PM Eric Wong <e@80x24.org> wrote:
> It's a bit much for common cases with git-send-email and
> reasonable MUAs, I think.  I don't know if formail is commonly
> installed, nowadays...

Of course ;-) You need it to run checkpatch on patch series obtained
through "b4 am", before you apply them to your tree:

$ cat *mbx | formail -s scripts/checkpatch.pl

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-29 22:16   ` Jonathan Corbet
@ 2021-11-30 13:10     ` Thorsten Leemhuis
  2021-12-01 12:24       ` Geert Uytterhoeven
  0 siblings, 1 reply; 32+ messages in thread
From: Thorsten Leemhuis @ 2021-11-30 13:10 UTC (permalink / raw)
  To: Jonathan Corbet, workflows
  Cc: Linux Kernel Mailing List, Konstantin Ryabitsev

On 29.11.21 23:16, Jonathan Corbet wrote:
> Thorsten Leemhuis <linux@leemhuis.info> writes:
> 
>> Introduce the tags 'Reported:' and 'Reviewed:' in addition to 'Link:',
>> as the latter is overloaded and hence doesn't indicate what the provided
>> URL is about. Documenting these also provides clarity, as a few
>> developers have used 'References:' to point to problem reports;
>> nevertheless 'Reported:' was chosen for this purpose, as it perfectly
>> matches up with the 'Reported-by:' tag commonly used already and needed
>> in this situation already.
>>
>> Signed-off-by: Thorsten Leemhuis <linux@leemhuis.info>
>> To: workflows@vger.kernel.org
> 
> Thanks for flooding my inbox during a holiday week :)

We didn't have one here. :-D But hey, it wasn't out of envy, I'm a
little ignorant to local holiday/festival seasons, too.

>  Just looking at this now.
> 
>> v1/RFC:
>> - first, *rough version* to see how this idea is received in the
>>   community
>> ---
>>  Documentation/maintainer/configure-git.rst   |  6 +--
>>  Documentation/process/5.Posting.rst          | 54 ++++++++++++++------
>>  Documentation/process/submitting-patches.rst | 22 ++++----
>>  3 files changed, 53 insertions(+), 29 deletions(-)
>>
>> diff --git a/Documentation/maintainer/configure-git.rst b/Documentation/maintainer/configure-git.rst
>> index 80ae5030a590..8429d45d661c 100644
>> --- a/Documentation/maintainer/configure-git.rst
>> +++ b/Documentation/maintainer/configure-git.rst
>> @@ -40,12 +40,12 @@ Creating commit links to lore.kernel.org
>>  The web site http://lore.kernel.org is meant as a grand archive of all mail
>>  list traffic concerning or influencing the kernel development. Storing archives
>>  of patches here is a recommended practice, and when a maintainer applies a
>> -patch to a subsystem tree, it is a good idea to provide a Link: tag with a
>> +patch to a subsystem tree, it is a good idea to provide a Reviewed: tag with a
>>  reference back to the lore archive so that people that browse the commit
>>  history can find related discussions and rationale behind a certain change.
>>  The link tag will look like this:
>>  
>> -    Link: https://lore.kernel.org/r/<message-id>
>> +    Reviewed: https://lore.kernel.org/r/<message-id>
> 
> The *link* tag will look like that?

Ha, good point, will fix that.

> [...]
>> +The tags in common use are:
>> +
>> + - ``Reported:`` points to a report of a problem fixed by this patch. The
>> +   provided URL thus might point to a entry in a bug tracker or a mail in a
>> +   mailing list archive. Typically this tag is followed by a "Reported-by:"
>> +   tag (see below).
>> +
>> + - ``Link:`` points to websites providing additional backgrounds or details,
>> +   for example a document with a specification implemented by the patch.
> 
> So this is a serious change from how Link: is used now, and runs counter
> to the scripts used by a lot of maintainers.  I suspect that this thread
> is only as short as it is because a lot of people haven't seen this yet;
> it could be a hard change to sell.

Yeah, I'm aware of that. And to be honest: I don't have a strong
interest in this, just think it might be the right thing to do. And I
just got the impression that regzbot's dependence on the Link: tag for
linking to regression reports is making the ambiguity of the tag worse.
That lead to the thought: well, simply bring it up now and see what
people think; if they don't like it, I can tell myself "well, I tried to
improve it, but it was not welcomed" and sleep well at night. At least
as long as my cat allows me to. :-)

> Also, I think that documents like specs should be called out separately
> in the changelog, with text saying what they actually are.

I wonder a little if that is worth the trouble, but hey, why not, fine
with me.

>> + - ``Reviewed:`` ignore this, as maintainers add it when applying a patch, to
>> +   make the commit point to the latest public review of the patch.
> 
> Another question would be: what's the interplay between the (quite
> similar) "Reviewed" and "Reviewed-by" tags (and the same for the report
> tags).

Hmmm, I liked the interplay for Reported/Reported-by, but yeah, for
Reviewed/Reviewed-by I see the problem now.

>  If there's a "Reviewed" do we still need "Reviewed-by"?  That
> should be spelled out, whichever way is wanted.

I didn't want to undermine or obsolete "Reviewed-by" at all. I sometimes
wonder if this and "Tested-by" should be stored somewhere else (in "git
notes" or something), so they can be extended after a change got
committed -- but that's a whole different topic and something I'm even
less interested in driving forward. :-D

Maybe "Reviewed" was simply the wrong term. Maybe "Review:", "Posted:",
or "MergeRequest:" would be better in general and avoid this problem.

> I do worry that the similarity is going to lead to a certain amount of
> confusion and use of the wrong tag.  People have a hard time getting all
> the tags we have now right; adding more that look almost like the
> existing ones seems like a recipe for trouble.
> 
> For these reasons, I would be more inclined toward Konstantin's
> suggestion of adding notes to the existing Link: tags.

Yeah, maybe, but that results in long lines and forces people to type more.

>> +A third kind of tags are used to document which developers were involved in
>> +the development of the patch. Each of these uses this format::
>>  
>>  	tag: Full Name <email address>  optional-other-stuff
>>  
>>  The tags in common use are:
>>  
>> - - Signed-off-by: this is a developer's certification that he or she has
>> + - ``Signed-off-by:`` is a developer's certification that he or she has
> 
> So this markup addition is a separate change that would belong in its
> own patch.

Okay.

>  Do we really need it, though?  It clutters the text and
> irritates the anti-RST minority (which has been mercifully quiet
> recently) without really adding any benefit.

I'm not strongly attached to it. I added it after I noticed that it's at
least for me slightly unclear if the colon is part of the tag or used to
precede the explanation for the tag (in the 'Signed-off-by' case it's
both at the same time). And after adding the proposed tags the html view
imho looked a lot better.

Thx for the feedback!

Ciao, Thorsten

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-30 13:10     ` Thorsten Leemhuis
@ 2021-12-01 12:24       ` Geert Uytterhoeven
  0 siblings, 0 replies; 32+ messages in thread
From: Geert Uytterhoeven @ 2021-12-01 12:24 UTC (permalink / raw)
  To: Thorsten Leemhuis
  Cc: Jonathan Corbet, workflows, Linux Kernel Mailing List,
	Konstantin Ryabitsev

Hi Thorsten,

On Wed, Dec 1, 2021 at 7:32 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
> On 29.11.21 23:16, Jonathan Corbet wrote:
> > Thorsten Leemhuis <linux@leemhuis.info> writes:
> >> Introduce the tags 'Reported:' and 'Reviewed:' in addition to 'Link:',
> >> as the latter is overloaded and hence doesn't indicate what the provided
> >> URL is about. Documenting these also provides clarity, as a few
> >> developers have used 'References:' to point to problem reports;
> >> nevertheless 'Reported:' was chosen for this purpose, as it perfectly
> >> matches up with the 'Reported-by:' tag commonly used already and needed
> >> in this situation already.
> >>
> >> Signed-off-by: Thorsten Leemhuis <linux@leemhuis.info>

> > [...]
> >> +The tags in common use are:
> >> +
> >> + - ``Reported:`` points to a report of a problem fixed by this patch. The
> >> +   provided URL thus might point to a entry in a bug tracker or a mail in a
> >> +   mailing list archive. Typically this tag is followed by a "Reported-by:"
> >> +   tag (see below).
> >> +
> >> + - ``Link:`` points to websites providing additional backgrounds or details,
> >> +   for example a document with a specification implemented by the patch.
> >
> > So this is a serious change from how Link: is used now, and runs counter
> > to the scripts used by a lot of maintainers.  I suspect that this thread
> > is only as short as it is because a lot of people haven't seen this yet;
> > it could be a hard change to sell.

I saw it, but decided to wait a bit for other input...

> Yeah, I'm aware of that. And to be honest: I don't have a strong
> interest in this, just think it might be the right thing to do. And I
> just got the impression that regzbot's dependence on the Link: tag for
> linking to regression reports is making the ambiguity of the tag worse.
> That lead to the thought: well, simply bring it up now and see what
> people think; if they don't like it, I can tell myself "well, I tried to
> improve it, but it was not welcomed" and sleep well at night. At least
> as long as my cat allows me to. :-)
>
> > Also, I think that documents like specs should be called out separately
> > in the changelog, with text saying what they actually are.
>
> I wonder a little if that is worth the trouble, but hey, why not, fine
> with me.
>
> >> + - ``Reviewed:`` ignore this, as maintainers add it when applying a patch, to
> >> +   make the commit point to the latest public review of the patch.
> >
> > Another question would be: what's the interplay between the (quite
> > similar) "Reviewed" and "Reviewed-by" tags (and the same for the report
> > tags).
>
> Hmmm, I liked the interplay for Reported/Reported-by, but yeah, for
> Reviewed/Reviewed-by I see the problem now.
>
> >  If there's a "Reviewed" do we still need "Reviewed-by"?  That
> > should be spelled out, whichever way is wanted.
>
> I didn't want to undermine or obsolete "Reviewed-by" at all. I sometimes
> wonder if this and "Tested-by" should be stored somewhere else (in "git
> notes" or something), so they can be extended after a change got
> committed -- but that's a whole different topic and something I'm even
> less interested in driving forward. :-D
>
> Maybe "Reviewed" was simply the wrong term. Maybe "Review:", "Posted:",
> or "MergeRequest:" would be better in general and avoid this problem.
>
> > I do worry that the similarity is going to lead to a certain amount of
> > confusion and use of the wrong tag.  People have a hard time getting all
> > the tags we have now right; adding more that look almost like the
> > existing ones seems like a recipe for trouble.
> >
> > For these reasons, I would be more inclined toward Konstantin's
> > suggestion of adding notes to the existing Link: tags.

Exactly. The power of the "Link" tag is that it can refer to a
variety of related content. I.e. the meaning is derived from the
link target, which can be an email discussion, a bug report, a bug
tracker page, ...

A proliferation of tags complicates life for patch authors and commit
analyzers. IMHO adding tags should only be done as a last resort, as
it doesn't come without a cost.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-11-30  8:24                   ` Geert Uytterhoeven
@ 2021-12-08 13:41                     ` Thorsten Leemhuis
  2021-12-08 17:02                       ` Eric Wong
  0 siblings, 1 reply; 32+ messages in thread
From: Thorsten Leemhuis @ 2021-12-08 13:41 UTC (permalink / raw)
  To: Geert Uytterhoeven, Eric Wong
  Cc: Jani Nikula, Junio C Hamano, workflows,
	Linux Kernel Mailing List, Konstantin Ryabitsev, Jonathan Corbet,
	Git Mailing List, Linus Walleij, Kees Cook

Hi Eric!

On 30.11.21 09:24, Geert Uytterhoeven wrote:
> On Mon, Nov 29, 2021 at 11:29 PM Eric Wong <e@80x24.org> wrote:
>> It's a bit much for common cases with git-send-email and
>> reasonable MUAs, I think.  I don't know if formail is commonly
>> installed, nowadays...

Well, after your earlier suggestion I considered to go with this:

-	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Link:
https://lore.kernel.org/r/$1|g;' "$1"
+	perl -pi -e 's|^Message-ID:\s*<?([^>]+)>?$|Link:
https://lore.kernel.org/r/$1|i;' "$1"

But...

> Of course ;-) You need it to run checkpatch on patch series obtained
> through "b4 am", before you apply them to your tree:
> 
> $ cat *mbx | formail -s scripts/checkpatch.pl

...this made me wonder if formail would be the better solution. I came
up with this:

formail -A "Link: https://lore.kernel.org/r/`formail -c -x Message-ID <
"${1}" | sed 's!.*<\(.*\)>!\1!'`" < "${1}" | sponge "${1}"

Downsides: instead of perl it requires sed and sponge (part of
moreutils, which I guess not everyone has installed; but I tried to
avoid a big here document or moving files around).

Is that worth it? Or is there a way to realize this in a more elegant
fashion with tools everyone has installed?

Ciao, Thorsten

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

* Re: [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:'
  2021-12-08 13:41                     ` Thorsten Leemhuis
@ 2021-12-08 17:02                       ` Eric Wong
  0 siblings, 0 replies; 32+ messages in thread
From: Eric Wong @ 2021-12-08 17:02 UTC (permalink / raw)
  To: Thorsten Leemhuis
  Cc: Geert Uytterhoeven, Jani Nikula, Junio C Hamano, workflows,
	linux-kernel, Konstantin Ryabitsev, Jonathan Corbet, git,
	Linus Walleij, Kees Cook

Thorsten Leemhuis <linux@leemhuis.info> wrote:
> On 30.11.21 09:24, Geert Uytterhoeven wrote:
> > On Mon, Nov 29, 2021 at 11:29 PM Eric Wong <e@80x24.org> wrote:
> >> It's a bit much for common cases with git-send-email and
> >> reasonable MUAs, I think.  I don't know if formail is commonly
> >> installed, nowadays...
> 
> Well, after your earlier suggestion I considered to go with this:
> 
> -	perl -pi -e 's|^Message-Id:\s*<?([^>]+)>?$|Link:
> https://lore.kernel.org/r/$1|g;' "$1"
> +	perl -pi -e 's|^Message-ID:\s*<?([^>]+)>?$|Link:
> https://lore.kernel.org/r/$1|i;' "$1"
> 
> But...
> 
> > Of course ;-) You need it to run checkpatch on patch series obtained
> > through "b4 am", before you apply them to your tree:
> > 
> > $ cat *mbx | formail -s scripts/checkpatch.pl
> 
> ...this made me wonder if formail would be the better solution. I came
> up with this:
> 
> formail -A "Link: https://lore.kernel.org/r/`formail -c -x Message-ID <
> "${1}" | sed 's!.*<\(.*\)>!\1!'`" < "${1}" | sponge "${1}"
> 
> Downsides: instead of perl it requires sed and sponge (part of
> moreutils, which I guess not everyone has installed; but I tried to
> avoid a big here document or moving files around).

As Geert noted, formail is probably reasonable, but I certainly
don't have moreutils across all the systems I'm using right now.

> Is that worth it? Or is there a way to realize this in a more elegant
> fashion with tools everyone has installed?

*shrug*  Since newlines after ':' are a concern and it's (probably :P)
safe to slurp entire contents of emails into memory nowadays;
some minor tweaks to the original perl invocation should work:

* use `$/ = undef' to force Perl to operate on the entire input at once
* use `m' RE modifier to ensure `^' and `$' still match SOL/EOL
  ($/ is only the input record separator, it doesn't change
   Perl's definition of "lines" for `^' and `$')

perl -i -p -e 'BEGIN{$/=undef};s|^Message-ID:\s*<?([^>]+)>?$|Link:
 https://lore.kernel.org/r/$1|im;'

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

end of thread, other threads:[~2021-12-08 17:02 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-22  7:33 [RFC PATCH v1 0/1] Create 'Reported:' and 'Reviewed:' tags for links in commit messages Thorsten Leemhuis
2021-11-22  7:33 ` [RFC PATCH v1 1/1] docs: add the new commit-msg tags 'Reported:' and 'Reviewed:' Thorsten Leemhuis
2021-11-22 16:29   ` Steven Rostedt
2021-11-22 18:50     ` Thorsten Leemhuis
2021-11-22 20:24       ` Steven Rostedt
2021-11-23  8:53         ` Thorsten Leemhuis
2021-11-23 18:52   ` Eric Wong
2021-11-24  1:37     ` Junio C Hamano
2021-11-24  6:12       ` Eric Wong
2021-11-26 12:49       ` Ævar Arnfjörð Bjarmason
2021-11-24  2:08     ` Ævar Arnfjörð Bjarmason
2021-11-26  7:29     ` Thorsten Leemhuis
2021-11-26 17:11       ` Eric Wong
2021-11-27 19:32         ` Thorsten Leemhuis
2021-11-27 19:52           ` Eric Wong
2021-11-27 20:20             ` Junio C Hamano
2021-11-29 12:03               ` Jani Nikula
2021-11-29 17:10                 ` Steven Rostedt
2021-11-29 17:18                 ` Junio C Hamano
2021-11-29 19:18                   ` Jani Nikula
2021-11-29 17:26                 ` Eric Wong
2021-11-29 19:20                   ` Jani Nikula
2021-11-30  8:24                   ` Geert Uytterhoeven
2021-12-08 13:41                     ` Thorsten Leemhuis
2021-12-08 17:02                       ` Eric Wong
2021-11-29 22:16   ` Jonathan Corbet
2021-11-30 13:10     ` Thorsten Leemhuis
2021-12-01 12:24       ` Geert Uytterhoeven
2021-11-22 15:12 ` [RFC PATCH v1 0/1] Create 'Reported:' and 'Reviewed:' tags for links in commit messages Konstantin Ryabitsev
2021-11-22 17:04   ` Steven Rostedt
2021-11-22 18:40     ` Thorsten Leemhuis
2021-11-22 18:48       ` Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).