All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] docs: Document the Link: tag formally
@ 2019-12-16  9:38 Linus Walleij
  2019-12-16 13:14 ` Jani Nikula
  0 siblings, 1 reply; 12+ messages in thread
From: Linus Walleij @ 2019-12-16  9:38 UTC (permalink / raw)
  To: Jonathan Corbet; +Cc: linux-doc, Linus Walleij, Russell King

We have a lot of Link: tags in commits these days and they are
not formally defined in the kernel documentation. Let's put
a separate paragraph about it in submitting-patches.rst where
most other tags are defined.

Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Russell King <linux@armlinux.org.uk>
Reported-by: Russell King <linux@armlinux.org.uk>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 Documentation/process/submitting-patches.rst | 21 ++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
index ba5e944c7a63..20ef984aa743 100644
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -643,9 +643,22 @@ which stable kernel versions should receive your fix. This is the preferred
 method for indicating a bug fixed by the patch. See :ref:`describe_changes`
 for more details.
 
+14) Link: tags
+--------------
+
+A Link: attribute can be used to provide a link back to a protocol of a
+discussion pertaining to the patch. A typical link looks like this:
+
+    Link: https://lore.kernel.org/r/<message-id>
+
+Any HTTP[S] links can be referenced. It is customary for maintainers to add
+Link: tags to reference discussions on mailing lists, and this can be done
+automatically with the git tool when applying patches in mailbox format, see
+:ref:`Documentation/maintainer/configure-git.rst <configure git>`.
+
 .. _the_canonical_patch_format:
 
-14) The canonical patch format
+15) The canonical patch format
 ------------------------------
 
 This section describes how the patch itself should be formatted.  Note
@@ -768,7 +781,7 @@ references.
 
 .. _explicit_in_reply_to:
 
-15) Explicit In-Reply-To headers
+16) Explicit In-Reply-To headers
 --------------------------------
 
 It can be helpful to manually add In-Reply-To: headers to a patch
@@ -782,7 +795,7 @@ helpful, you can use the https://lkml.kernel.org/ redirector (e.g., in
 the cover email text) to link to an earlier version of the patch series.
 
 
-16) Providing base tree information
+17) Providing base tree information
 -----------------------------------
 
 When other developers receive your patches and start the review process,
@@ -833,7 +846,7 @@ either below the ``---`` line or at the very bottom of all other
 content, right before your email signature.
 
 
-17) Sending ``git pull`` requests
+18) Sending ``git pull`` requests
 ---------------------------------
 
 If you have a series of patches, it may be most convenient to have the
-- 
2.23.0


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

* Re: [PATCH] docs: Document the Link: tag formally
  2019-12-16  9:38 [PATCH] docs: Document the Link: tag formally Linus Walleij
@ 2019-12-16 13:14 ` Jani Nikula
  2019-12-16 13:33   ` Russell King - ARM Linux admin
  0 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2019-12-16 13:14 UTC (permalink / raw)
  To: Linus Walleij, Jonathan Corbet; +Cc: linux-doc, Linus Walleij, Russell King

On Mon, 16 Dec 2019, Linus Walleij <linus.walleij@linaro.org> wrote:
> We have a lot of Link: tags in commits these days and they are
> not formally defined in the kernel documentation. Let's put
> a separate paragraph about it in submitting-patches.rst where
> most other tags are defined.
>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Russell King <linux@armlinux.org.uk>
> Reported-by: Russell King <linux@armlinux.org.uk>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  Documentation/process/submitting-patches.rst | 21 ++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> index ba5e944c7a63..20ef984aa743 100644
> --- a/Documentation/process/submitting-patches.rst
> +++ b/Documentation/process/submitting-patches.rst
> @@ -643,9 +643,22 @@ which stable kernel versions should receive your fix. This is the preferred
>  method for indicating a bug fixed by the patch. See :ref:`describe_changes`
>  for more details.
>  
> +14) Link: tags
> +--------------
> +
> +A Link: attribute can be used to provide a link back to a protocol of a
> +discussion pertaining to the patch. A typical link looks like this:
> +
> +    Link: https://lore.kernel.org/r/<message-id>
> +
> +Any HTTP[S] links can be referenced. It is customary for maintainers to add
> +Link: tags to reference discussions on mailing lists, and this can be done
> +automatically with the git tool when applying patches in mailbox format, see
> +:ref:`Documentation/maintainer/configure-git.rst <configure git>`.

I'd like to emphasize even more strongly that it is applied by the
maintainer or committer, and should reference the patch that got
applied. And that the patch submitters shouldn't try to add it
themselves. (Which makes you wonder about the placement in
submitting-patches.rst.) IMO other references should use References:
that is already widely used.

The above would also match current usage in e.g. the drm subsystem.

> +
>  .. _the_canonical_patch_format:
>  
> -14) The canonical patch format
> +15) The canonical patch format

Would it be about time we dropped the numbers?


BR,
Jani.

>  ------------------------------
>  
>  This section describes how the patch itself should be formatted.  Note
> @@ -768,7 +781,7 @@ references.
>  
>  .. _explicit_in_reply_to:
>  
> -15) Explicit In-Reply-To headers
> +16) Explicit In-Reply-To headers
>  --------------------------------
>  
>  It can be helpful to manually add In-Reply-To: headers to a patch
> @@ -782,7 +795,7 @@ helpful, you can use the https://lkml.kernel.org/ redirector (e.g., in
>  the cover email text) to link to an earlier version of the patch series.
>  
>  
> -16) Providing base tree information
> +17) Providing base tree information
>  -----------------------------------
>  
>  When other developers receive your patches and start the review process,
> @@ -833,7 +846,7 @@ either below the ``---`` line or at the very bottom of all other
>  content, right before your email signature.
>  
>  
> -17) Sending ``git pull`` requests
> +18) Sending ``git pull`` requests
>  ---------------------------------
>  
>  If you have a series of patches, it may be most convenient to have the

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [PATCH] docs: Document the Link: tag formally
  2019-12-16 13:14 ` Jani Nikula
@ 2019-12-16 13:33   ` Russell King - ARM Linux admin
  2019-12-16 14:02     ` Jani Nikula
  0 siblings, 1 reply; 12+ messages in thread
From: Russell King - ARM Linux admin @ 2019-12-16 13:33 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Linus Walleij, Jonathan Corbet, linux-doc

On Mon, Dec 16, 2019 at 03:14:56PM +0200, Jani Nikula wrote:
> On Mon, 16 Dec 2019, Linus Walleij <linus.walleij@linaro.org> wrote:
> > We have a lot of Link: tags in commits these days and they are
> > not formally defined in the kernel documentation. Let's put
> > a separate paragraph about it in submitting-patches.rst where
> > most other tags are defined.
> >
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: Russell King <linux@armlinux.org.uk>
> > Reported-by: Russell King <linux@armlinux.org.uk>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> >  Documentation/process/submitting-patches.rst | 21 ++++++++++++++++----
> >  1 file changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> > index ba5e944c7a63..20ef984aa743 100644
> > --- a/Documentation/process/submitting-patches.rst
> > +++ b/Documentation/process/submitting-patches.rst
> > @@ -643,9 +643,22 @@ which stable kernel versions should receive your fix. This is the preferred
> >  method for indicating a bug fixed by the patch. See :ref:`describe_changes`
> >  for more details.
> >  
> > +14) Link: tags
> > +--------------
> > +
> > +A Link: attribute can be used to provide a link back to a protocol of a
> > +discussion pertaining to the patch. A typical link looks like this:
> > +
> > +    Link: https://lore.kernel.org/r/<message-id>
> > +
> > +Any HTTP[S] links can be referenced. It is customary for maintainers to add
> > +Link: tags to reference discussions on mailing lists, and this can be done
> > +automatically with the git tool when applying patches in mailbox format, see
> > +:ref:`Documentation/maintainer/configure-git.rst <configure git>`.
> 
> I'd like to emphasize even more strongly that it is applied by the
> maintainer or committer, and should reference the patch that got
> applied. And that the patch submitters shouldn't try to add it
> themselves. (Which makes you wonder about the placement in
> submitting-patches.rst.) IMO other references should use References:
> that is already widely used.

I'm the maintainer of phylink.  During discussions, I may propose a
patch for someone to try.  When successful, I'll send a new email
submitting the patch officially to davem as the networking maintainer
as an entirely separate thread.

Using Link: to the patch that was submitted officially is obviously
impossible, but you would want to link to the discussion that resulted
in the patch, rather than the official submission - which would
generally be the submission plus an "applied" reply.

Looking at the history between v5.4 and current, it seems that it's
only DRM that uses References, and there's variations in its
formatting.  Some references to commits contain the word "commit"
and others do not.  Some references, from what I can tell, are
useless - for example "HSDES#1405586840" which I guess is some kind
of internal system somewhere.

There are some broken Link:s:

https://patchwork.freedesktop.org/patch/msgid/
<CACAvsv56Am90okV334eXgxDuK228sb9UJxMiOYjNAMShvvv4cg@mail.gmail.com

That has a space in there with a "<" - which suggests a broken script
is in use that mis-parses the email.

The References: thing is also completely undocumented...

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

* Re: [PATCH] docs: Document the Link: tag formally
  2019-12-16 13:33   ` Russell King - ARM Linux admin
@ 2019-12-16 14:02     ` Jani Nikula
  2019-12-16 14:16       ` Russell King - ARM Linux admin
  0 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2019-12-16 14:02 UTC (permalink / raw)
  To: Russell King - ARM Linux admin; +Cc: Linus Walleij, Jonathan Corbet, linux-doc

On Mon, 16 Dec 2019, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> On Mon, Dec 16, 2019 at 03:14:56PM +0200, Jani Nikula wrote:
>> On Mon, 16 Dec 2019, Linus Walleij <linus.walleij@linaro.org> wrote:
>> > We have a lot of Link: tags in commits these days and they are
>> > not formally defined in the kernel documentation. Let's put
>> > a separate paragraph about it in submitting-patches.rst where
>> > most other tags are defined.
>> >
>> > Cc: Jonathan Corbet <corbet@lwn.net>
>> > Cc: Russell King <linux@armlinux.org.uk>
>> > Reported-by: Russell King <linux@armlinux.org.uk>
>> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> > ---
>> >  Documentation/process/submitting-patches.rst | 21 ++++++++++++++++----
>> >  1 file changed, 17 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
>> > index ba5e944c7a63..20ef984aa743 100644
>> > --- a/Documentation/process/submitting-patches.rst
>> > +++ b/Documentation/process/submitting-patches.rst
>> > @@ -643,9 +643,22 @@ which stable kernel versions should receive your fix. This is the preferred
>> >  method for indicating a bug fixed by the patch. See :ref:`describe_changes`
>> >  for more details.
>> >  
>> > +14) Link: tags
>> > +--------------
>> > +
>> > +A Link: attribute can be used to provide a link back to a protocol of a
>> > +discussion pertaining to the patch. A typical link looks like this:
>> > +
>> > +    Link: https://lore.kernel.org/r/<message-id>
>> > +
>> > +Any HTTP[S] links can be referenced. It is customary for maintainers to add
>> > +Link: tags to reference discussions on mailing lists, and this can be done
>> > +automatically with the git tool when applying patches in mailbox format, see
>> > +:ref:`Documentation/maintainer/configure-git.rst <configure git>`.
>> 
>> I'd like to emphasize even more strongly that it is applied by the
>> maintainer or committer, and should reference the patch that got
>> applied. And that the patch submitters shouldn't try to add it
>> themselves. (Which makes you wonder about the placement in
>> submitting-patches.rst.) IMO other references should use References:
>> that is already widely used.
>
> I'm the maintainer of phylink.  During discussions, I may propose a
> patch for someone to try.  When successful, I'll send a new email
> submitting the patch officially to davem as the networking maintainer
> as an entirely separate thread.
>
> Using Link: to the patch that was submitted officially is obviously
> impossible, but you would want to link to the discussion that resulted
> in the patch, rather than the official submission - which would
> generally be the submission plus an "applied" reply.

If there are N versions of the patch with discussion that eventually
lead to the final version that was applied, which one of the previous
patch versions or discussions do you think should be linked to? IMO the
only one that actually is unambiguous is the patch that got applied. IMO
only patches that were submitted to a mailing list should be applied.

I the case of i915, the links will point at patchwork that will also
contain testing results for the patch. It's a paper trail of sorts.

> Looking at the history between v5.4 and current, it seems that it's
> only DRM that uses References, and there's variations in its
> formatting.  Some references to commits contain the word "commit"
> and others do not.  Some references, from what I can tell, are
> useless - for example "HSDES#1405586840" which I guess is some kind
> of internal system somewhere.
>
> There are some broken Link:s:
>
> https://patchwork.freedesktop.org/patch/msgid/
> <CACAvsv56Am90okV334eXgxDuK228sb9UJxMiOYjNAMShvvv4cg@mail.gmail.com
>
> That has a space in there with a "<" - which suggests a broken script
> is in use that mis-parses the email.

Appears to be on a merge commit with likely manually added Link:
tag. Should be fixed, but hardly evidence of anything really relevant.

> The References: thing is also completely undocumented...

Agreed, needs documenting as well.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [PATCH] docs: Document the Link: tag formally
  2019-12-16 14:02     ` Jani Nikula
@ 2019-12-16 14:16       ` Russell King - ARM Linux admin
  2019-12-16 14:31         ` Jani Nikula
  0 siblings, 1 reply; 12+ messages in thread
From: Russell King - ARM Linux admin @ 2019-12-16 14:16 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Linus Walleij, Jonathan Corbet, linux-doc

On Mon, Dec 16, 2019 at 04:02:04PM +0200, Jani Nikula wrote:
> On Mon, 16 Dec 2019, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> > On Mon, Dec 16, 2019 at 03:14:56PM +0200, Jani Nikula wrote:
> >> On Mon, 16 Dec 2019, Linus Walleij <linus.walleij@linaro.org> wrote:
> >> > We have a lot of Link: tags in commits these days and they are
> >> > not formally defined in the kernel documentation. Let's put
> >> > a separate paragraph about it in submitting-patches.rst where
> >> > most other tags are defined.
> >> >
> >> > Cc: Jonathan Corbet <corbet@lwn.net>
> >> > Cc: Russell King <linux@armlinux.org.uk>
> >> > Reported-by: Russell King <linux@armlinux.org.uk>
> >> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> >> > ---
> >> >  Documentation/process/submitting-patches.rst | 21 ++++++++++++++++----
> >> >  1 file changed, 17 insertions(+), 4 deletions(-)
> >> >
> >> > diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> >> > index ba5e944c7a63..20ef984aa743 100644
> >> > --- a/Documentation/process/submitting-patches.rst
> >> > +++ b/Documentation/process/submitting-patches.rst
> >> > @@ -643,9 +643,22 @@ which stable kernel versions should receive your fix. This is the preferred
> >> >  method for indicating a bug fixed by the patch. See :ref:`describe_changes`
> >> >  for more details.
> >> >  
> >> > +14) Link: tags
> >> > +--------------
> >> > +
> >> > +A Link: attribute can be used to provide a link back to a protocol of a
> >> > +discussion pertaining to the patch. A typical link looks like this:
> >> > +
> >> > +    Link: https://lore.kernel.org/r/<message-id>
> >> > +
> >> > +Any HTTP[S] links can be referenced. It is customary for maintainers to add
> >> > +Link: tags to reference discussions on mailing lists, and this can be done
> >> > +automatically with the git tool when applying patches in mailbox format, see
> >> > +:ref:`Documentation/maintainer/configure-git.rst <configure git>`.
> >> 
> >> I'd like to emphasize even more strongly that it is applied by the
> >> maintainer or committer, and should reference the patch that got
> >> applied. And that the patch submitters shouldn't try to add it
> >> themselves. (Which makes you wonder about the placement in
> >> submitting-patches.rst.) IMO other references should use References:
> >> that is already widely used.
> >
> > I'm the maintainer of phylink.  During discussions, I may propose a
> > patch for someone to try.  When successful, I'll send a new email
> > submitting the patch officially to davem as the networking maintainer
> > as an entirely separate thread.
> >
> > Using Link: to the patch that was submitted officially is obviously
> > impossible, but you would want to link to the discussion that resulted
> > in the patch, rather than the official submission - which would
> > generally be the submission plus an "applied" reply.
> 
> If there are N versions of the patch with discussion that eventually
> lead to the final version that was applied, which one of the previous
> patch versions or discussions do you think should be linked to? IMO the
> only one that actually is unambiguous is the patch that got applied. IMO
> only patches that were submitted to a mailing list should be applied.

Even if it's just the patch submission and an "applied" reply.  To me,
that seems to be just noise.

My point with the above scenario is that the discussion that resulted
in the patch being proposed is the relevant discussion that needs to
be linked to the patch - which will include the context about why the
patch was generated and its testing.  The official submission will
contain none of that.

I guess what this is highlighting is that different people have
different ways of working, and trying to force one thing (Link:) to
be used with one way of working doesn't scale across the whole kernel.

So, stating that Link: should only be added by a maintainer or
committer will have a detrimental effect on workflows that do not
match the one you have in mind.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

* Re: [PATCH] docs: Document the Link: tag formally
  2019-12-16 14:16       ` Russell King - ARM Linux admin
@ 2019-12-16 14:31         ` Jani Nikula
  2019-12-16 14:43           ` Russell King - ARM Linux admin
  0 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2019-12-16 14:31 UTC (permalink / raw)
  To: Russell King - ARM Linux admin; +Cc: Linus Walleij, Jonathan Corbet, linux-doc

On Mon, 16 Dec 2019, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> On Mon, Dec 16, 2019 at 04:02:04PM +0200, Jani Nikula wrote:
>> On Mon, 16 Dec 2019, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
>> > On Mon, Dec 16, 2019 at 03:14:56PM +0200, Jani Nikula wrote:
>> >> On Mon, 16 Dec 2019, Linus Walleij <linus.walleij@linaro.org> wrote:
>> >> > We have a lot of Link: tags in commits these days and they are
>> >> > not formally defined in the kernel documentation. Let's put
>> >> > a separate paragraph about it in submitting-patches.rst where
>> >> > most other tags are defined.
>> >> >
>> >> > Cc: Jonathan Corbet <corbet@lwn.net>
>> >> > Cc: Russell King <linux@armlinux.org.uk>
>> >> > Reported-by: Russell King <linux@armlinux.org.uk>
>> >> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> >> > ---
>> >> >  Documentation/process/submitting-patches.rst | 21 ++++++++++++++++----
>> >> >  1 file changed, 17 insertions(+), 4 deletions(-)
>> >> >
>> >> > diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
>> >> > index ba5e944c7a63..20ef984aa743 100644
>> >> > --- a/Documentation/process/submitting-patches.rst
>> >> > +++ b/Documentation/process/submitting-patches.rst
>> >> > @@ -643,9 +643,22 @@ which stable kernel versions should receive your fix. This is the preferred
>> >> >  method for indicating a bug fixed by the patch. See :ref:`describe_changes`
>> >> >  for more details.
>> >> >  
>> >> > +14) Link: tags
>> >> > +--------------
>> >> > +
>> >> > +A Link: attribute can be used to provide a link back to a protocol of a
>> >> > +discussion pertaining to the patch. A typical link looks like this:
>> >> > +
>> >> > +    Link: https://lore.kernel.org/r/<message-id>
>> >> > +
>> >> > +Any HTTP[S] links can be referenced. It is customary for maintainers to add
>> >> > +Link: tags to reference discussions on mailing lists, and this can be done
>> >> > +automatically with the git tool when applying patches in mailbox format, see
>> >> > +:ref:`Documentation/maintainer/configure-git.rst <configure git>`.
>> >> 
>> >> I'd like to emphasize even more strongly that it is applied by the
>> >> maintainer or committer, and should reference the patch that got
>> >> applied. And that the patch submitters shouldn't try to add it
>> >> themselves. (Which makes you wonder about the placement in
>> >> submitting-patches.rst.) IMO other references should use References:
>> >> that is already widely used.
>> >
>> > I'm the maintainer of phylink.  During discussions, I may propose a
>> > patch for someone to try.  When successful, I'll send a new email
>> > submitting the patch officially to davem as the networking maintainer
>> > as an entirely separate thread.
>> >
>> > Using Link: to the patch that was submitted officially is obviously
>> > impossible, but you would want to link to the discussion that resulted
>> > in the patch, rather than the official submission - which would
>> > generally be the submission plus an "applied" reply.
>> 
>> If there are N versions of the patch with discussion that eventually
>> lead to the final version that was applied, which one of the previous
>> patch versions or discussions do you think should be linked to? IMO the
>> only one that actually is unambiguous is the patch that got applied. IMO
>> only patches that were submitted to a mailing list should be applied.
>
> Even if it's just the patch submission and an "applied" reply.  To me,
> that seems to be just noise.
>
> My point with the above scenario is that the discussion that resulted
> in the patch being proposed is the relevant discussion that needs to
> be linked to the patch - which will include the context about why the
> patch was generated and its testing.  The official submission will
> contain none of that.
>
> I guess what this is highlighting is that different people have
> different ways of working, and trying to force one thing (Link:) to
> be used with one way of working doesn't scale across the whole kernel.
>
> So, stating that Link: should only be added by a maintainer or
> committer will have a detrimental effect on workflows that do not
> match the one you have in mind.

Basically this boils down to the question whether it is generally more
useful to have a Link: tag with a very specific meaning and potentially
*other* less specific additional tags for other purposes, or to not
define a specific meaning to the Link: tag at all.

I promote the former, with e.g. References: used for more free form
references. I'm really fine with either, but with the latter I'll just
filter out submitter added Link: tags and turn them to References: or
something in my corner of the kernel. (Meaning that lax Link: tags will
have a detrimental effect on workflows that do not match the one you
have in mind... ;)

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [PATCH] docs: Document the Link: tag formally
  2019-12-16 14:31         ` Jani Nikula
@ 2019-12-16 14:43           ` Russell King - ARM Linux admin
  2019-12-16 15:13             ` Jani Nikula
  0 siblings, 1 reply; 12+ messages in thread
From: Russell King - ARM Linux admin @ 2019-12-16 14:43 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Linus Walleij, Jonathan Corbet, linux-doc

On Mon, Dec 16, 2019 at 04:31:52PM +0200, Jani Nikula wrote:
> On Mon, 16 Dec 2019, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> > On Mon, Dec 16, 2019 at 04:02:04PM +0200, Jani Nikula wrote:
> >> On Mon, 16 Dec 2019, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> >> > On Mon, Dec 16, 2019 at 03:14:56PM +0200, Jani Nikula wrote:
> >> >> On Mon, 16 Dec 2019, Linus Walleij <linus.walleij@linaro.org> wrote:
> >> >> > We have a lot of Link: tags in commits these days and they are
> >> >> > not formally defined in the kernel documentation. Let's put
> >> >> > a separate paragraph about it in submitting-patches.rst where
> >> >> > most other tags are defined.
> >> >> >
> >> >> > Cc: Jonathan Corbet <corbet@lwn.net>
> >> >> > Cc: Russell King <linux@armlinux.org.uk>
> >> >> > Reported-by: Russell King <linux@armlinux.org.uk>
> >> >> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> >> >> > ---
> >> >> >  Documentation/process/submitting-patches.rst | 21 ++++++++++++++++----
> >> >> >  1 file changed, 17 insertions(+), 4 deletions(-)
> >> >> >
> >> >> > diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> >> >> > index ba5e944c7a63..20ef984aa743 100644
> >> >> > --- a/Documentation/process/submitting-patches.rst
> >> >> > +++ b/Documentation/process/submitting-patches.rst
> >> >> > @@ -643,9 +643,22 @@ which stable kernel versions should receive your fix. This is the preferred
> >> >> >  method for indicating a bug fixed by the patch. See :ref:`describe_changes`
> >> >> >  for more details.
> >> >> >  
> >> >> > +14) Link: tags
> >> >> > +--------------
> >> >> > +
> >> >> > +A Link: attribute can be used to provide a link back to a protocol of a
> >> >> > +discussion pertaining to the patch. A typical link looks like this:
> >> >> > +
> >> >> > +    Link: https://lore.kernel.org/r/<message-id>
> >> >> > +
> >> >> > +Any HTTP[S] links can be referenced. It is customary for maintainers to add
> >> >> > +Link: tags to reference discussions on mailing lists, and this can be done
> >> >> > +automatically with the git tool when applying patches in mailbox format, see
> >> >> > +:ref:`Documentation/maintainer/configure-git.rst <configure git>`.
> >> >> 
> >> >> I'd like to emphasize even more strongly that it is applied by the
> >> >> maintainer or committer, and should reference the patch that got
> >> >> applied. And that the patch submitters shouldn't try to add it
> >> >> themselves. (Which makes you wonder about the placement in
> >> >> submitting-patches.rst.) IMO other references should use References:
> >> >> that is already widely used.
> >> >
> >> > I'm the maintainer of phylink.  During discussions, I may propose a
> >> > patch for someone to try.  When successful, I'll send a new email
> >> > submitting the patch officially to davem as the networking maintainer
> >> > as an entirely separate thread.
> >> >
> >> > Using Link: to the patch that was submitted officially is obviously
> >> > impossible, but you would want to link to the discussion that resulted
> >> > in the patch, rather than the official submission - which would
> >> > generally be the submission plus an "applied" reply.
> >> 
> >> If there are N versions of the patch with discussion that eventually
> >> lead to the final version that was applied, which one of the previous
> >> patch versions or discussions do you think should be linked to? IMO the
> >> only one that actually is unambiguous is the patch that got applied. IMO
> >> only patches that were submitted to a mailing list should be applied.
> >
> > Even if it's just the patch submission and an "applied" reply.  To me,
> > that seems to be just noise.
> >
> > My point with the above scenario is that the discussion that resulted
> > in the patch being proposed is the relevant discussion that needs to
> > be linked to the patch - which will include the context about why the
> > patch was generated and its testing.  The official submission will
> > contain none of that.
> >
> > I guess what this is highlighting is that different people have
> > different ways of working, and trying to force one thing (Link:) to
> > be used with one way of working doesn't scale across the whole kernel.
> >
> > So, stating that Link: should only be added by a maintainer or
> > committer will have a detrimental effect on workflows that do not
> > match the one you have in mind.
> 
> Basically this boils down to the question whether it is generally more
> useful to have a Link: tag with a very specific meaning and potentially
> *other* less specific additional tags for other purposes, or to not
> define a specific meaning to the Link: tag at all.
> 
> I promote the former, with e.g. References: used for more free form
> references. I'm really fine with either, but with the latter I'll just
> filter out submitter added Link: tags and turn them to References: or
> something in my corner of the kernel. (Meaning that lax Link: tags will
> have a detrimental effect on workflows that do not match the one you
> have in mind... ;)

Well, I've just done it again:

- Patch submitted sends a patch series that uses some of my code
- I review the series, and spot deficiencies in it which can be
  solved by patching my code to add additional functionality.
- I create a patch for my code, and send it as a reply to that
  thread.  The discussion in the thread is entirely relevant to
  the patch.
- They review it and decide whether or not it solves their problem
  and whether they want to use that approach.
- If successful, I send it "officially" as an entirely separate
  thread.
- Maybe Andrew Lunn reviews it and sends a reviewed-by tag.
- DaveM applies it and sends an "applied" message.

With Link: pointing at the "official" submission, all the context
behind why the patch was necessary gets lost.

With Link: pointing at the email in the original discussion that
contained the patch proposal, the discussion for the patch (including
all the things that you're talking about) get properly linked to.
So, I would much rather have Link: pointing to this discussion rather
than a useless "it was reviewed and applied" set of emails, which we
know anyway from the attributation tags that are on the commit.

I guess with your workflow is along the lines of:

- Patch gets officially submitted
- Patch gets reviewed and some discussion happens
- Result of the discussion is to apply the patch
- Patch gets applied with Link: to the official submission

which does seem appropriate to ensure that the Link: tag is added
when the patch is applied.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

* Re: [PATCH] docs: Document the Link: tag formally
  2019-12-16 14:43           ` Russell King - ARM Linux admin
@ 2019-12-16 15:13             ` Jani Nikula
  2019-12-16 16:02               ` Russell King - ARM Linux admin
  2019-12-16 16:22               ` Jonathan Corbet
  0 siblings, 2 replies; 12+ messages in thread
From: Jani Nikula @ 2019-12-16 15:13 UTC (permalink / raw)
  To: Russell King - ARM Linux admin; +Cc: Linus Walleij, Jonathan Corbet, linux-doc

On Mon, 16 Dec 2019, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> On Mon, Dec 16, 2019 at 04:31:52PM +0200, Jani Nikula wrote:
>> On Mon, 16 Dec 2019, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
>> > On Mon, Dec 16, 2019 at 04:02:04PM +0200, Jani Nikula wrote:
>> >> On Mon, 16 Dec 2019, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
>> >> > On Mon, Dec 16, 2019 at 03:14:56PM +0200, Jani Nikula wrote:
>> >> >> On Mon, 16 Dec 2019, Linus Walleij <linus.walleij@linaro.org> wrote:
>> >> >> > We have a lot of Link: tags in commits these days and they are
>> >> >> > not formally defined in the kernel documentation. Let's put
>> >> >> > a separate paragraph about it in submitting-patches.rst where
>> >> >> > most other tags are defined.
>> >> >> >
>> >> >> > Cc: Jonathan Corbet <corbet@lwn.net>
>> >> >> > Cc: Russell King <linux@armlinux.org.uk>
>> >> >> > Reported-by: Russell King <linux@armlinux.org.uk>
>> >> >> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> >> >> > ---
>> >> >> >  Documentation/process/submitting-patches.rst | 21 ++++++++++++++++----
>> >> >> >  1 file changed, 17 insertions(+), 4 deletions(-)
>> >> >> >
>> >> >> > diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
>> >> >> > index ba5e944c7a63..20ef984aa743 100644
>> >> >> > --- a/Documentation/process/submitting-patches.rst
>> >> >> > +++ b/Documentation/process/submitting-patches.rst
>> >> >> > @@ -643,9 +643,22 @@ which stable kernel versions should receive your fix. This is the preferred
>> >> >> >  method for indicating a bug fixed by the patch. See :ref:`describe_changes`
>> >> >> >  for more details.
>> >> >> >  
>> >> >> > +14) Link: tags
>> >> >> > +--------------
>> >> >> > +
>> >> >> > +A Link: attribute can be used to provide a link back to a protocol of a
>> >> >> > +discussion pertaining to the patch. A typical link looks like this:
>> >> >> > +
>> >> >> > +    Link: https://lore.kernel.org/r/<message-id>
>> >> >> > +
>> >> >> > +Any HTTP[S] links can be referenced. It is customary for maintainers to add
>> >> >> > +Link: tags to reference discussions on mailing lists, and this can be done
>> >> >> > +automatically with the git tool when applying patches in mailbox format, see
>> >> >> > +:ref:`Documentation/maintainer/configure-git.rst <configure git>`.
>> >> >> 
>> >> >> I'd like to emphasize even more strongly that it is applied by the
>> >> >> maintainer or committer, and should reference the patch that got
>> >> >> applied. And that the patch submitters shouldn't try to add it
>> >> >> themselves. (Which makes you wonder about the placement in
>> >> >> submitting-patches.rst.) IMO other references should use References:
>> >> >> that is already widely used.
>> >> >
>> >> > I'm the maintainer of phylink.  During discussions, I may propose a
>> >> > patch for someone to try.  When successful, I'll send a new email
>> >> > submitting the patch officially to davem as the networking maintainer
>> >> > as an entirely separate thread.
>> >> >
>> >> > Using Link: to the patch that was submitted officially is obviously
>> >> > impossible, but you would want to link to the discussion that resulted
>> >> > in the patch, rather than the official submission - which would
>> >> > generally be the submission plus an "applied" reply.
>> >> 
>> >> If there are N versions of the patch with discussion that eventually
>> >> lead to the final version that was applied, which one of the previous
>> >> patch versions or discussions do you think should be linked to? IMO the
>> >> only one that actually is unambiguous is the patch that got applied. IMO
>> >> only patches that were submitted to a mailing list should be applied.
>> >
>> > Even if it's just the patch submission and an "applied" reply.  To me,
>> > that seems to be just noise.
>> >
>> > My point with the above scenario is that the discussion that resulted
>> > in the patch being proposed is the relevant discussion that needs to
>> > be linked to the patch - which will include the context about why the
>> > patch was generated and its testing.  The official submission will
>> > contain none of that.
>> >
>> > I guess what this is highlighting is that different people have
>> > different ways of working, and trying to force one thing (Link:) to
>> > be used with one way of working doesn't scale across the whole kernel.
>> >
>> > So, stating that Link: should only be added by a maintainer or
>> > committer will have a detrimental effect on workflows that do not
>> > match the one you have in mind.
>> 
>> Basically this boils down to the question whether it is generally more
>> useful to have a Link: tag with a very specific meaning and potentially
>> *other* less specific additional tags for other purposes, or to not
>> define a specific meaning to the Link: tag at all.
>> 
>> I promote the former, with e.g. References: used for more free form
>> references. I'm really fine with either, but with the latter I'll just
>> filter out submitter added Link: tags and turn them to References: or
>> something in my corner of the kernel. (Meaning that lax Link: tags will
>> have a detrimental effect on workflows that do not match the one you
>> have in mind... ;)
>
> Well, I've just done it again:
>
> - Patch submitted sends a patch series that uses some of my code
> - I review the series, and spot deficiencies in it which can be
>   solved by patching my code to add additional functionality.
> - I create a patch for my code, and send it as a reply to that
>   thread.  The discussion in the thread is entirely relevant to
>   the patch.
> - They review it and decide whether or not it solves their problem
>   and whether they want to use that approach.
> - If successful, I send it "officially" as an entirely separate
>   thread.
> - Maybe Andrew Lunn reviews it and sends a reviewed-by tag.
> - DaveM applies it and sends an "applied" message.
>
> With Link: pointing at the "official" submission, all the context
> behind why the patch was necessary gets lost.
>
> With Link: pointing at the email in the original discussion that
> contained the patch proposal, the discussion for the patch (including
> all the things that you're talking about) get properly linked to.
> So, I would much rather have Link: pointing to this discussion rather
> than a useless "it was reviewed and applied" set of emails, which we
> know anyway from the attributation tags that are on the commit.
>
> I guess with your workflow is along the lines of:
>
> - Patch gets officially submitted
> - Patch gets reviewed and some discussion happens
> - Result of the discussion is to apply the patch
> - Patch gets applied with Link: to the official submission
>
> which does seem appropriate to ensure that the Link: tag is added
> when the patch is applied.

I did understand your workflow from the start, thank you. That's not the
problem.

I'm not sure that you understand our workflow. But it doesn't matter;
it's clear that they are different. In your workflow, the value for the
tag is subjective. In our workflow, it's not, it's unambiguous. And
there is really no point in debating the relative merits of the
workflows.

The *only* question is, whether we should both use the tag Foo: for the
different meanings and different workflows, or whether one should use
Foo: and the other should use Bar:.


BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [PATCH] docs: Document the Link: tag formally
  2019-12-16 15:13             ` Jani Nikula
@ 2019-12-16 16:02               ` Russell King - ARM Linux admin
  2019-12-17 10:55                 ` Jani Nikula
  2019-12-16 16:22               ` Jonathan Corbet
  1 sibling, 1 reply; 12+ messages in thread
From: Russell King - ARM Linux admin @ 2019-12-16 16:02 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Linus Walleij, Jonathan Corbet, linux-doc

On Mon, Dec 16, 2019 at 05:13:20PM +0200, Jani Nikula wrote:
> On Mon, 16 Dec 2019, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> > On Mon, Dec 16, 2019 at 04:31:52PM +0200, Jani Nikula wrote:
> >> On Mon, 16 Dec 2019, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> >> > On Mon, Dec 16, 2019 at 04:02:04PM +0200, Jani Nikula wrote:
> >> >> On Mon, 16 Dec 2019, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> >> >> > On Mon, Dec 16, 2019 at 03:14:56PM +0200, Jani Nikula wrote:
> >> >> >> On Mon, 16 Dec 2019, Linus Walleij <linus.walleij@linaro.org> wrote:
> >> >> >> > We have a lot of Link: tags in commits these days and they are
> >> >> >> > not formally defined in the kernel documentation. Let's put
> >> >> >> > a separate paragraph about it in submitting-patches.rst where
> >> >> >> > most other tags are defined.
> >> >> >> >
> >> >> >> > Cc: Jonathan Corbet <corbet@lwn.net>
> >> >> >> > Cc: Russell King <linux@armlinux.org.uk>
> >> >> >> > Reported-by: Russell King <linux@armlinux.org.uk>
> >> >> >> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> >> >> >> > ---
> >> >> >> >  Documentation/process/submitting-patches.rst | 21 ++++++++++++++++----
> >> >> >> >  1 file changed, 17 insertions(+), 4 deletions(-)
> >> >> >> >
> >> >> >> > diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> >> >> >> > index ba5e944c7a63..20ef984aa743 100644
> >> >> >> > --- a/Documentation/process/submitting-patches.rst
> >> >> >> > +++ b/Documentation/process/submitting-patches.rst
> >> >> >> > @@ -643,9 +643,22 @@ which stable kernel versions should receive your fix. This is the preferred
> >> >> >> >  method for indicating a bug fixed by the patch. See :ref:`describe_changes`
> >> >> >> >  for more details.
> >> >> >> >  
> >> >> >> > +14) Link: tags
> >> >> >> > +--------------
> >> >> >> > +
> >> >> >> > +A Link: attribute can be used to provide a link back to a protocol of a
> >> >> >> > +discussion pertaining to the patch. A typical link looks like this:
> >> >> >> > +
> >> >> >> > +    Link: https://lore.kernel.org/r/<message-id>
> >> >> >> > +
> >> >> >> > +Any HTTP[S] links can be referenced. It is customary for maintainers to add
> >> >> >> > +Link: tags to reference discussions on mailing lists, and this can be done
> >> >> >> > +automatically with the git tool when applying patches in mailbox format, see
> >> >> >> > +:ref:`Documentation/maintainer/configure-git.rst <configure git>`.
> >> >> >> 
> >> >> >> I'd like to emphasize even more strongly that it is applied by the
> >> >> >> maintainer or committer, and should reference the patch that got
> >> >> >> applied. And that the patch submitters shouldn't try to add it
> >> >> >> themselves. (Which makes you wonder about the placement in
> >> >> >> submitting-patches.rst.) IMO other references should use References:
> >> >> >> that is already widely used.
> >> >> >
> >> >> > I'm the maintainer of phylink.  During discussions, I may propose a
> >> >> > patch for someone to try.  When successful, I'll send a new email
> >> >> > submitting the patch officially to davem as the networking maintainer
> >> >> > as an entirely separate thread.
> >> >> >
> >> >> > Using Link: to the patch that was submitted officially is obviously
> >> >> > impossible, but you would want to link to the discussion that resulted
> >> >> > in the patch, rather than the official submission - which would
> >> >> > generally be the submission plus an "applied" reply.
> >> >> 
> >> >> If there are N versions of the patch with discussion that eventually
> >> >> lead to the final version that was applied, which one of the previous
> >> >> patch versions or discussions do you think should be linked to? IMO the
> >> >> only one that actually is unambiguous is the patch that got applied. IMO
> >> >> only patches that were submitted to a mailing list should be applied.
> >> >
> >> > Even if it's just the patch submission and an "applied" reply.  To me,
> >> > that seems to be just noise.
> >> >
> >> > My point with the above scenario is that the discussion that resulted
> >> > in the patch being proposed is the relevant discussion that needs to
> >> > be linked to the patch - which will include the context about why the
> >> > patch was generated and its testing.  The official submission will
> >> > contain none of that.
> >> >
> >> > I guess what this is highlighting is that different people have
> >> > different ways of working, and trying to force one thing (Link:) to
> >> > be used with one way of working doesn't scale across the whole kernel.
> >> >
> >> > So, stating that Link: should only be added by a maintainer or
> >> > committer will have a detrimental effect on workflows that do not
> >> > match the one you have in mind.
> >> 
> >> Basically this boils down to the question whether it is generally more
> >> useful to have a Link: tag with a very specific meaning and potentially
> >> *other* less specific additional tags for other purposes, or to not
> >> define a specific meaning to the Link: tag at all.
> >> 
> >> I promote the former, with e.g. References: used for more free form
> >> references. I'm really fine with either, but with the latter I'll just
> >> filter out submitter added Link: tags and turn them to References: or
> >> something in my corner of the kernel. (Meaning that lax Link: tags will
> >> have a detrimental effect on workflows that do not match the one you
> >> have in mind... ;)
> >
> > Well, I've just done it again:
> >
> > - Patch submitted sends a patch series that uses some of my code
> > - I review the series, and spot deficiencies in it which can be
> >   solved by patching my code to add additional functionality.
> > - I create a patch for my code, and send it as a reply to that
> >   thread.  The discussion in the thread is entirely relevant to
> >   the patch.
> > - They review it and decide whether or not it solves their problem
> >   and whether they want to use that approach.
> > - If successful, I send it "officially" as an entirely separate
> >   thread.
> > - Maybe Andrew Lunn reviews it and sends a reviewed-by tag.
> > - DaveM applies it and sends an "applied" message.
> >
> > With Link: pointing at the "official" submission, all the context
> > behind why the patch was necessary gets lost.
> >
> > With Link: pointing at the email in the original discussion that
> > contained the patch proposal, the discussion for the patch (including
> > all the things that you're talking about) get properly linked to.
> > So, I would much rather have Link: pointing to this discussion rather
> > than a useless "it was reviewed and applied" set of emails, which we
> > know anyway from the attributation tags that are on the commit.
> >
> > I guess with your workflow is along the lines of:
> >
> > - Patch gets officially submitted
> > - Patch gets reviewed and some discussion happens
> > - Result of the discussion is to apply the patch
> > - Patch gets applied with Link: to the official submission
> >
> > which does seem appropriate to ensure that the Link: tag is added
> > when the patch is applied.
> 
> I did understand your workflow from the start, thank you. That's not the
> problem.
> 
> I'm not sure that you understand our workflow. But it doesn't matter;
> it's clear that they are different. In your workflow, the value for the
> tag is subjective. In our workflow, it's not, it's unambiguous. And
> there is really no point in debating the relative merits of the
> workflows.

I don't see why you think that it's subjective in my workflow. It
isn't. The proposed patch and the officially submitted patch are
generally identical. The difference is, which thread is the more
useful thread to link the patch to - the one with the discussion
about the patch including why it's needed, or the official submission
that contains none of that.

I guess it depends whether you wish to capture the submission itself
or the actual useful discussion about the patch with the tag. If
we're going to be officially defining this tag, then really that
question needs to be settled.

What worries me at this point is that you seem to want to withdraw
from the discussion before any concensus has been reached, which
presents a problem for Linus' proposed patch...

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

* Re: [PATCH] docs: Document the Link: tag formally
  2019-12-16 15:13             ` Jani Nikula
  2019-12-16 16:02               ` Russell King - ARM Linux admin
@ 2019-12-16 16:22               ` Jonathan Corbet
  2019-12-16 20:36                 ` Theodore Y. Ts'o
  1 sibling, 1 reply; 12+ messages in thread
From: Jonathan Corbet @ 2019-12-16 16:22 UTC (permalink / raw)
  To: Jani Nikula; +Cc: Russell King - ARM Linux admin, Linus Walleij, linux-doc

On Mon, 16 Dec 2019 17:13:20 +0200
Jani Nikula <jani.nikula@linux.intel.com> wrote:

> The *only* question is, whether we should both use the tag Foo: for the
> different meanings and different workflows, or whether one should use
> Foo: and the other should use Bar:.

This is, of course, a part of the wider discussion on patch tracking IDs
and such; I kind of doubt that this relatively small group can resolve
it for the community as a whole.  It seems to be agreed that it's good
for a patch submission to reference previous postings; I'm not sure that
we've decided on Link: as the way to do that.

That creates a bit of a problem for me; I don't want to be trying to
create community policy through the docs; I'd rather accept patches that
I know reflect an existing consensus.

The practice of using Link: tags to refer to previous discussions is well
established in the TIP tree neighborhood, if nowhere else.  So there are
precedents for using it the way Russell is wanting to.  Multiple tags can
reflect the discussion at various points.

I do hesitate a bit to put this into submitting-patches.rst for a couple
of reasons, though.  One is that I'm not at all sure we want to encourage
submitters to add these tags; it sounds like a recipe for maintainers
having to follow them all and decide which ones really belong there, and
maintainers don't really need more to do.  That document is already *way*
too long, to the point that it's a barrier that new contributors have to
overcome; I worry about making it even longer.

Probably we'll end up adding something like this.  I do think I would
agree with Jani, though, that we should at least lightly discourage
developers from adding these tags all over the place.

Sorry for the stream-of-consciousness dump...

jon

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

* Re: [PATCH] docs: Document the Link: tag formally
  2019-12-16 16:22               ` Jonathan Corbet
@ 2019-12-16 20:36                 ` Theodore Y. Ts'o
  0 siblings, 0 replies; 12+ messages in thread
From: Theodore Y. Ts'o @ 2019-12-16 20:36 UTC (permalink / raw)
  To: Jonathan Corbet
  Cc: Jani Nikula, Russell King - ARM Linux admin, Linus Walleij, linux-doc

On Mon, Dec 16, 2019 at 09:22:55AM -0700, Jonathan Corbet wrote:
> On Mon, 16 Dec 2019 17:13:20 +0200
> Jani Nikula <jani.nikula@linux.intel.com> wrote:
> 
> > The *only* question is, whether we should both use the tag Foo: for the
> > different meanings and different workflows, or whether one should use
> > Foo: and the other should use Bar:.
> 
> This is, of course, a part of the wider discussion on patch tracking IDs
> and such; I kind of doubt that this relatively small group can resolve
> it for the community as a whole.  It seems to be agreed that it's good
> for a patch submission to reference previous postings; I'm not sure that
> we've decided on Link: as the way to do that.

I believe that at least amongst the maintainers at the Maintainer's
Summit and on the follow-on workflows@vger.kernel.org, there is
general consensus for *maintainers* to add a

	Link: https://lore.kernel.org/r/...

tag to the patch that they are accepting into their trees.  There are
even some git hook scripts being circulated which automate this.

What there isn't consensus yet on is a way for patch *submitters* to
reference earler versions of a particular patch.  There is one school
of thought saying that you just match on subject lines, looking for
V<n-1>: commit description to find it.  There are others who don't
like this, because it requires manual searching work, and it doesn't
handle the case where a patch gets split, or merged, or where the
patch description is modified for clarity's sake.

Another school of thought is just keeping the V1, V2, V3, etc.,
versions of the patch as replies to earlier one, so they can be found
via e-mail chaining.  Some people believe this gets ugly, especially
for patches which go through a large number of revisions, and for
patch series with a large number of patches.


What *I've* been doing for ext4 is in the Vn+1 update of a patch, I've
been manually adding a:

Previous-Version-Link: https://lore.kernel.org/r/...

to the commit before mailing it out, and trusting that the maintainer
(me, myself, and I :-) will strip out the Previous-Version-Link: and
adding a Link: trailer to the final version on the mailing list.

This allows someone who is doing code archeology to find the mailing
list reference via the Link: tag in the git tree, and that version of
the patch in the mailing list archive will have a
Previous-Version-Link in the previous version of the commit, going all
the way back to the V1 version of the commit.  That is, the
Previous-Version-Link: trailer never appears in the git commit which
is pushed to Linus; but it *is* in all but the first version of the
patch in the mailing list.

I think this scheme is perfect, without flaw, and Solomonic in meeting
the needs for people to be able to find all of the various versions of
the patch and its discussions while not leaving extra hair in the git
commit found in the repository.  But, I'd also be the first to admit
that it's not general community consensus at this point.  :-)


> That creates a bit of a problem for me; I don't want to be trying to
> create community policy through the docs; I'd rather accept patches that
> I know reflect an existing consensus.

Yeah, I think it's too early at this point for anythig in
submitting-patches.  It may or may not be too early to put in the git
hook automation for automatically adding a pointer to the version of
the patch which the maintainer is actually pulling her repository
using "git am", though.

I'd also suggest redirecting discussions about how to annotate commits
to find previous versions of patches to the workflows@vger.kernel.org
list; that's where most of the discussions on this topic have taken
place since Lisbon.

Cheers,

					- Ted

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

* Re: [PATCH] docs: Document the Link: tag formally
  2019-12-16 16:02               ` Russell King - ARM Linux admin
@ 2019-12-17 10:55                 ` Jani Nikula
  0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2019-12-17 10:55 UTC (permalink / raw)
  To: Russell King - ARM Linux admin; +Cc: Linus Walleij, Jonathan Corbet, linux-doc

On Mon, 16 Dec 2019, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> On Mon, Dec 16, 2019 at 05:13:20PM +0200, Jani Nikula wrote:
>> I'm not sure that you understand our workflow. But it doesn't matter;
>> it's clear that they are different. In your workflow, the value for the
>> tag is subjective. In our workflow, it's not, it's unambiguous. And
>> there is really no point in debating the relative merits of the
>> workflows.
>
> I don't see why you think that it's subjective in my workflow. It
> isn't. The proposed patch and the officially submitted patch are
> generally identical. The difference is, which thread is the more
> useful thread to link the patch to - the one with the discussion
> about the patch including why it's needed, or the official submission
> that contains none of that.

In my experience, it's common to have several versions of a patch, often
in several threads. If I were to choose which message to reference,
other than the patch that was actually applied, I'd feel it would be a
subjective choice.

> I guess it depends whether you wish to capture the submission itself
> or the actual useful discussion about the patch with the tag. If
> we're going to be officially defining this tag, then really that
> question needs to be settled.
>
> What worries me at this point is that you seem to want to withdraw
> from the discussion before any concensus has been reached, which
> presents a problem for Linus' proposed patch...

I don't think we can reach a meaningful consensus between ourselves that
other people would care about anyway; indeed this sentiment is echoed in
Jon's and Ted's replies.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

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

end of thread, other threads:[~2019-12-17 10:55 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-16  9:38 [PATCH] docs: Document the Link: tag formally Linus Walleij
2019-12-16 13:14 ` Jani Nikula
2019-12-16 13:33   ` Russell King - ARM Linux admin
2019-12-16 14:02     ` Jani Nikula
2019-12-16 14:16       ` Russell King - ARM Linux admin
2019-12-16 14:31         ` Jani Nikula
2019-12-16 14:43           ` Russell King - ARM Linux admin
2019-12-16 15:13             ` Jani Nikula
2019-12-16 16:02               ` Russell King - ARM Linux admin
2019-12-17 10:55                 ` Jani Nikula
2019-12-16 16:22               ` Jonathan Corbet
2019-12-16 20:36                 ` Theodore Y. Ts'o

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