[PATCH 0/3] Clarifications around Acked-by and "# Suffix" proposal

[PATCH 0/3] Clarifications around Acked-by and "# Suffix" proposal

[Miguel Ojeda]

A patch series that aims to clarify what the purpose of the Acked-by tag is,
in order to help newcomers understand the kernel process better, as well as
to give extra flexibility on the usage of the Acked-by tag.

It formalizes one of the proposals from [1]: "# Suffix" for Acked-by.

I hope this helps!

Link: https://lore.kernel.org/rust-for-linux/CANiq72m4fea15Z0fFZauz8N2madkBJ0G7Dc094OwoajnXmROOA@mail.gmail.com/ [1]

Miguel Ojeda (3):
  docs: submitting-patches: clarify Acked-by and introduce "# Suffix"
  docs: submitting-patches: clarify difference between Acked-by and
    Reviewed-by
  docs: submitting-patches: clarify that signers may use their
    discretion on tags

 Documentation/process/submitting-patches.rst | 22 ++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)


base-commit: 9d89551994a430b50c4fffcb1e617a057fa76e20
--
2.48.0

[PATCH 1/3] docs: submitting-patches: clarify Acked-by and introduce "# Suffix"

[Miguel Ojeda]

Acked-by is typically used by maintainers. However, sometimes it is
useful to be able to accept the tag from other stakeholders that may not
have done a deep technical review or may not be kernel developers. For
instance:

  - People with domain knowledge, such as the original author of the
    code being modified.

  - Userspace-side reviewers for a kernel uAPI patch, like in DRM --
    see Documentation/gpu/drm-uapi.rst:

    > The userspace-side reviewer should also provide an Acked-by on the
    > kernel uAPI patch indicating that they believe the proposed uAPI
    > is sound and sufficiently documented and validated for userspace's
    > consumption.

  - Key users of a feature, such as in [1].

Thus clarify that Acked-by may be used by other stakeholders (but most
commonly by maintainers).

Since, in these cases, it may be confusing why an Acked-by is/was
provided, allow and suggest to provide a "# Suffix" explaining it.

The "# Suffix" for Acked-by is already being used to clarify what part
of the patch a maintainer is acknowledging, thus also mention "# Suffix"
in the relevant paragraph.

Link: https://lore.kernel.org/rust-for-linux/CANiq72m4fea15Z0fFZauz8N2madkBJ0G7Dc094OwoajnXmROOA@mail.gmail.com/ [1]
Acked-by: Shuah Khan <skhan@linuxfoundation.org>
Acked-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 Documentation/process/submitting-patches.rst | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
index 1518bd57adab..c7a28af235f7 100644
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -463,9 +463,17 @@ If a person was not directly involved in the preparation or handling of a
 patch but wishes to signify and record their approval of it then they can
 ask to have an Acked-by: line added to the patch's changelog.

-Acked-by: is often used by the maintainer of the affected code when that
+Acked-by: is meant to be used by those responsible for or involved with the
+affected code in one way or another.  Most commonly, the maintainer when that
 maintainer neither contributed to nor forwarded the patch.

+Acked-by: may also be used by other stakeholders, such as people with domain
+knowledge (e.g. the original author of the code being modified), userspace-side
+reviewers for a kernel uAPI patch or key users of a feature.  Optionally, in
+these cases, it can be useful to add a "# Suffix" to clarify its meaning::
+
+	Acked-by: The Stakeholder <stakeholder@example.org> # As primary user
+
 Acked-by: is not as formal as Signed-off-by:.  It is a record that the acker
 has at least reviewed the patch and has indicated acceptance.  Hence patch
 mergers will sometimes manually convert an acker's "yep, looks good to me"
@@ -477,7 +485,7 @@ For example, if a patch affects multiple subsystems and has an Acked-by: from
 one subsystem maintainer then this usually indicates acknowledgement of just
 the part which affects that maintainer's code.  Judgement should be used here.
 When in doubt people should refer to the original discussion in the mailing
-list archives.
+list archives.  A "# Suffix" may also be used in this case to clarify.

 If a person has had the opportunity to comment on a patch, but has not
 provided such comments, you may optionally add a ``Cc:`` tag to the patch.
--
2.48.0

[PATCH 2/3] docs: submitting-patches: clarify difference between Acked-by and Reviewed-by

[Miguel Ojeda]

Newcomers to the kernel need to learn the different tags that are
used in commit messages and when to apply them. Acked-by is sometimes
misunderstood, since the documentation did not really clarify (up to
the previous commit) when it should be used, especially compared to
Reviewed-by.

The previous commit already clarified who the usual providers of Acked-by
tags are, with examples. Thus provide a clarification paragraph for
the comparison with Reviewed-by, and give a couple examples reusing the
cases given above, in the previous commit.

Acked-by: Shuah Khan <skhan@linuxfoundation.org>
Acked-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 Documentation/process/submitting-patches.rst | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
index c7a28af235f7..7b0ac7370cb1 100644
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -480,6 +480,12 @@ mergers will sometimes manually convert an acker's "yep, looks good to me"
 into an Acked-by: (but note that it is usually better to ask for an
 explicit ack).
 
+Acked-by: is also less formal than Reviewed-by:.  For instance, maintainers may
+use it to signify that they are OK with a patch landing, but they may not have
+reviewed it as thoroughly as if a Reviewed-by: was provided.  Similarly, a key
+user may not have carried out a technical review of the patch, yet they may be
+satisfied with the general approach, the feature or the user-facing interface.
+
 Acked-by: does not necessarily indicate acknowledgement of the entire patch.
 For example, if a patch affects multiple subsystems and has an Acked-by: from
 one subsystem maintainer then this usually indicates acknowledgement of just
-- 
2.48.0

[PATCH 3/3] docs: submitting-patches: clarify that signers may use their discretion on tags

[Miguel Ojeda]

Tags are really appreciated by maintainers in general, since it means
someone is willing to put their name on a commit, be it as a reviewer,
tester, etc.

However, signers (i.e. submitters carrying tags from previous versions
and maintainers applying patches) may need to take or drop tags, on a
case-by-case basis, for different reasons.

Yet this is not explicitly spelled out in the documentation, thus there
may be instances [1] where contributors may feel unwelcome.

Thus, to clarify, state this clearly.

Link: https://lore.kernel.org/rust-for-linux/CAEg-Je-h4NitWb2ErFGCOqt0KQfXuyKWLhpnNHCdRzZdxi018Q@mail.gmail.com/ [1]
Suggested-by: Dan Williams <dan.j.williams@intel.com>
Acked-by: Shuah Khan <skhan@linuxfoundation.org>
Acked-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 Documentation/process/submitting-patches.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
index 7b0ac7370cb1..6754bc15f989 100644
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -614,6 +614,10 @@ process nor the requirement to Cc: stable@vger.kernel.org on all stable
 patch candidates. For more information, please read
 Documentation/process/stable-kernel-rules.rst.
 
+Finally, while providing tags is welcome and typically very appreciated, please
+note that signers (i.e. submitters and maintainers) may use their discretion in
+applying offered tags.
+
 .. _the_canonical_patch_format:
 
 The canonical patch format
-- 
2.48.0

Re: [PATCH 3/3] docs: submitting-patches: clarify that signers may use their discretion on tags

[Neal Gompa]

On Sun, Jan 12, 2025 at 10:30 AM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Tags are really appreciated by maintainers in general, since it means
> someone is willing to put their name on a commit, be it as a reviewer,
> tester, etc.
>
> However, signers (i.e. submitters carrying tags from previous versions
> and maintainers applying patches) may need to take or drop tags, on a
> case-by-case basis, for different reasons.
>
> Yet this is not explicitly spelled out in the documentation, thus there
> may be instances [1] where contributors may feel unwelcome.
>
> Thus, to clarify, state this clearly.
>
> Link: https://lore.kernel.org/rust-for-linux/CAEg-Je-h4NitWb2ErFGCOqt0KQfXuyKWLhpnNHCdRzZdxi018Q@mail.gmail.com/ [1]
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Acked-by: Shuah Khan <skhan@linuxfoundation.org>
> Acked-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
>  Documentation/process/submitting-patches.rst | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> index 7b0ac7370cb1..6754bc15f989 100644
> --- a/Documentation/process/submitting-patches.rst
> +++ b/Documentation/process/submitting-patches.rst
> @@ -614,6 +614,10 @@ process nor the requirement to Cc: stable@vger.kernel.org on all stable
>  patch candidates. For more information, please read
>  Documentation/process/stable-kernel-rules.rst.
>
> +Finally, while providing tags is welcome and typically very appreciated, please
> +note that signers (i.e. submitters and maintainers) may use their discretion in
> +applying offered tags.
> +
>  .. _the_canonical_patch_format:
>
>  The canonical patch format
> --
> 2.48.0
>

A tag must not be dropped without the tag submitter's authorization.
Otherwise it doesn't matter what you write here, the submitter *will*
feel unwelcome.

It is rude and discouraging to do so without their acceptance of doing so.


-- 
真実はいつも一つ！/ Always, there's only one truth!

Re: [PATCH 2/3] docs: submitting-patches: clarify difference between Acked-by and Reviewed-by

[Neal Gompa]

On Sun, Jan 12, 2025 at 10:30 AM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Newcomers to the kernel need to learn the different tags that are
> used in commit messages and when to apply them. Acked-by is sometimes
> misunderstood, since the documentation did not really clarify (up to
> the previous commit) when it should be used, especially compared to
> Reviewed-by.
>
> The previous commit already clarified who the usual providers of Acked-by
> tags are, with examples. Thus provide a clarification paragraph for
> the comparison with Reviewed-by, and give a couple examples reusing the
> cases given above, in the previous commit.
>
> Acked-by: Shuah Khan <skhan@linuxfoundation.org>
> Acked-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
>  Documentation/process/submitting-patches.rst | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> index c7a28af235f7..7b0ac7370cb1 100644
> --- a/Documentation/process/submitting-patches.rst
> +++ b/Documentation/process/submitting-patches.rst
> @@ -480,6 +480,12 @@ mergers will sometimes manually convert an acker's "yep, looks good to me"
>  into an Acked-by: (but note that it is usually better to ask for an
>  explicit ack).
>
> +Acked-by: is also less formal than Reviewed-by:.  For instance, maintainers may
> +use it to signify that they are OK with a patch landing, but they may not have
> +reviewed it as thoroughly as if a Reviewed-by: was provided.  Similarly, a key
> +user may not have carried out a technical review of the patch, yet they may be
> +satisfied with the general approach, the feature or the user-facing interface.
> +
>  Acked-by: does not necessarily indicate acknowledgement of the entire patch.
>  For example, if a patch affects multiple subsystems and has an Acked-by: from
>  one subsystem maintainer then this usually indicates acknowledgement of just
> --
> 2.48.0
>

This doesn't make sense as a distinction. What defines "thoroughly"?
To be honest, I think you should go the other way and become okay with
people sending Reviewed-by tags when people have looked over a patch
and consider it good to land.

To me, Acked-by mostly makes sense as a tag for people who *won't*
review the code, not for those who *will*. Blending Acked-by and
Reviewed-by just creates confusion.


-- 
真実はいつも一つ！/ Always, there's only one truth!

Re: [PATCH 1/3] docs: submitting-patches: clarify Acked-by and introduce "# Suffix"

[Neal Gompa]

On Sun, Jan 12, 2025 at 10:30 AM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Acked-by is typically used by maintainers. However, sometimes it is
> useful to be able to accept the tag from other stakeholders that may not
> have done a deep technical review or may not be kernel developers. For
> instance:
>
>   - People with domain knowledge, such as the original author of the
>     code being modified.
>
>   - Userspace-side reviewers for a kernel uAPI patch, like in DRM --
>     see Documentation/gpu/drm-uapi.rst:
>
>     > The userspace-side reviewer should also provide an Acked-by on the
>     > kernel uAPI patch indicating that they believe the proposed uAPI
>     > is sound and sufficiently documented and validated for userspace's
>     > consumption.
>
>   - Key users of a feature, such as in [1].
>
> Thus clarify that Acked-by may be used by other stakeholders (but most
> commonly by maintainers).
>
> Since, in these cases, it may be confusing why an Acked-by is/was
> provided, allow and suggest to provide a "# Suffix" explaining it.
>
> The "# Suffix" for Acked-by is already being used to clarify what part
> of the patch a maintainer is acknowledging, thus also mention "# Suffix"
> in the relevant paragraph.
>
> Link: https://lore.kernel.org/rust-for-linux/CANiq72m4fea15Z0fFZauz8N2madkBJ0G7Dc094OwoajnXmROOA@mail.gmail.com/ [1]
> Acked-by: Shuah Khan <skhan@linuxfoundation.org>
> Acked-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
>  Documentation/process/submitting-patches.rst | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> index 1518bd57adab..c7a28af235f7 100644
> --- a/Documentation/process/submitting-patches.rst
> +++ b/Documentation/process/submitting-patches.rst
> @@ -463,9 +463,17 @@ If a person was not directly involved in the preparation or handling of a
>  patch but wishes to signify and record their approval of it then they can
>  ask to have an Acked-by: line added to the patch's changelog.
>
> -Acked-by: is often used by the maintainer of the affected code when that
> +Acked-by: is meant to be used by those responsible for or involved with the
> +affected code in one way or another.  Most commonly, the maintainer when that
>  maintainer neither contributed to nor forwarded the patch.
>
> +Acked-by: may also be used by other stakeholders, such as people with domain
> +knowledge (e.g. the original author of the code being modified), userspace-side
> +reviewers for a kernel uAPI patch or key users of a feature.  Optionally, in
> +these cases, it can be useful to add a "# Suffix" to clarify its meaning::
> +
> +       Acked-by: The Stakeholder <stakeholder@example.org> # As primary user
> +
>  Acked-by: is not as formal as Signed-off-by:.  It is a record that the acker
>  has at least reviewed the patch and has indicated acceptance.  Hence patch
>  mergers will sometimes manually convert an acker's "yep, looks good to me"
> @@ -477,7 +485,7 @@ For example, if a patch affects multiple subsystems and has an Acked-by: from
>  one subsystem maintainer then this usually indicates acknowledgement of just
>  the part which affects that maintainer's code.  Judgement should be used here.
>  When in doubt people should refer to the original discussion in the mailing
> -list archives.
> +list archives.  A "# Suffix" may also be used in this case to clarify.
>
>  If a person has had the opportunity to comment on a patch, but has not
>  provided such comments, you may optionally add a ``Cc:`` tag to the patch.
> --
> 2.48.0

This makes sense to me.

Reviewed-by: Neal Gompa <neal@gompa.dev>



-- 
真実はいつも一つ！/ Always, there's only one truth!

Re: [PATCH 2/3] docs: submitting-patches: clarify difference between Acked-by and Reviewed-by

[Miguel Ojeda]

On Sun, Jan 12, 2025 at 4:51 PM Neal Gompa <neal@gompa.dev> wrote:
>
> This doesn't make sense as a distinction. What defines "thoroughly"?

It is a call, but when you give a Reviewed-by, it at least includes
what the "Reviewer's statement of oversight" mentions, unlike an
Acked-by.

> To be honest, I think you should go the other way and become okay with
> people sending Reviewed-by tags when people have looked over a patch
> and consider it good to land.

I am not sure what you mean. It is OK for people to send Reviewed-by
tags. The original discussion was about Acked-by because that is the
one that was usually used by maintainers only.

If what you mean is that Reviewed-by should not require an actual
review, then that is not the purpose of the tag. Please see the
"Reviewer's statement of oversight" -- its first bullet says:

     (a) I have carried out a technical review of this patch to
         evaluate its appropriateness and readiness for inclusion into
         the mainline kernel.

> To me, Acked-by mostly makes sense as a tag for people who *won't*
> review the code, not for those who *will*. Blending Acked-by and
> Reviewed-by just creates confusion.

The sentence about "thoroughly reviewing" in this patch is an example,
not the only use case. The next sentence gives another example that
explicitly says "may not have carried out a technical review".

This series tries to, precisely, widen the use cases of Acked-by and
explain those, so that it can be used by others who have not actually
carried out a technical review. Still, it is not meant to be used
randomly -- one is supposed to be a stakeholder in some way (please
see the previous patch).

Thanks for the quick review!

Cheers,
Miguel

Re: [PATCH 3/3] docs: submitting-patches: clarify that signers may use their discretion on tags

[Miguel Ojeda]

On Sun, Jan 12, 2025 at 4:47 PM Neal Gompa <neal@gompa.dev> wrote:
>
> A tag must not be dropped without the tag submitter's authorization.

Tags get routinely dropped, as the commit message explains. So we
cannot say "must not", sorry.

Even if that were not the case, tags are currently added/decided by
signers, not tag providers. This patch is trying to document how it
works today.

I understand it may feel bad to not have a tag applied, but it is
something normal in the current process. Tags are publicly archived
nevertheless, so e.g. one can still prove work to others.

I don't think misunderstandings about development processes mean
people is "rude": it may feel bad, but that does not mean others are
"rude" necessarily, unless they were rude in their manners or
something like that, which is not what is being discussed here.

Thanks for the quick review!

Cheers,
Miguel

Re: [PATCH 2/3] docs: submitting-patches: clarify difference between Acked-by and Reviewed-by

[Neal Gompa]

On Sun, Jan 12, 2025 at 11:31 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Sun, Jan 12, 2025 at 4:51 PM Neal Gompa <neal@gompa.dev> wrote:
> >
> > This doesn't make sense as a distinction. What defines "thoroughly"?
>
> It is a call, but when you give a Reviewed-by, it at least includes
> what the "Reviewer's statement of oversight" mentions, unlike an
> Acked-by.
>
> > To be honest, I think you should go the other way and become okay with
> > people sending Reviewed-by tags when people have looked over a patch
> > and consider it good to land.
>
> I am not sure what you mean. It is OK for people to send Reviewed-by
> tags. The original discussion was about Acked-by because that is the
> one that was usually used by maintainers only.
>
> If what you mean is that Reviewed-by should not require an actual
> review, then that is not the purpose of the tag. Please see the
> "Reviewer's statement of oversight" -- its first bullet says:
>
>      (a) I have carried out a technical review of this patch to
>          evaluate its appropriateness and readiness for inclusion into
>          the mainline kernel.
>

I've had my Reviewed-by tags silently ignored or deliberately stripped
because even though I've done a technical review, the maintainer does
not believe that I did. Therefore, what I am saying is that
maintainers seem to speciously decide whether an Acked-by or
Reviewed-by tag is appropriate or not *after* someone has sent it.

This is the fundamental problem I have right now. This decision is not
the maintainer's to make, it is the submitter's.


-- 
真実はいつも一つ！/ Always, there's only one truth!

Re: [PATCH 2/3] docs: submitting-patches: clarify difference between Acked-by and Reviewed-by

[Miguel Ojeda]

On Sun, Jan 12, 2025 at 5:36 PM Neal Gompa <neal@gompa.dev> wrote:
>
> I've had my Reviewed-by tags silently ignored or deliberately stripped
> because even though I've done a technical review, the maintainer does
> not believe that I did. Therefore, what I am saying is that
> maintainers seem to speciously decide whether an Acked-by or
> Reviewed-by tag is appropriate or not *after* someone has sent it.
>
> This is the fundamental problem I have right now. This decision is not
> the maintainer's to make, it is the submitter's.

First of all, if you are referring to other cases that I may not be
aware of, then I am sorry to hear that! Things like this can cause
friction sometimes.

Sending tags is OK, but the author/maintainer ultimately decides
whether to pick them up. For Reviewed-by, I would think it is rare
that it gets ignored -- after all, maintainers usually love to get
that tag.

If you mean the case that triggered this patch series, then (as far as
I understood it), the maintainer was OK with picking your Reviewed-by
and Tested-by if you sent it, but not your (bare) Acked-by. That
seemed reasonable to me, because the rules are a bit unclear around
the latter tag. This series tries to solve that part, so that in the
future one can provide an Acked-by with a suffix in a situation like
yours.

Cheers,
Miguel

Re: [PATCH 3/3] docs: submitting-patches: clarify that signers may use their discretion on tags

[Greg Kroah-Hartman]

On Sun, Jan 12, 2025 at 04:29:46PM +0100, Miguel Ojeda wrote:
> Tags are really appreciated by maintainers in general, since it means
> someone is willing to put their name on a commit, be it as a reviewer,
> tester, etc.
> 
> However, signers (i.e. submitters carrying tags from previous versions
> and maintainers applying patches) may need to take or drop tags, on a
> case-by-case basis, for different reasons.
> 
> Yet this is not explicitly spelled out in the documentation, thus there
> may be instances [1] where contributors may feel unwelcome.
> 
> Thus, to clarify, state this clearly.
> 
> Link: https://lore.kernel.org/rust-for-linux/CAEg-Je-h4NitWb2ErFGCOqt0KQfXuyKWLhpnNHCdRzZdxi018Q@mail.gmail.com/ [1]
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Acked-by: Shuah Khan <skhan@linuxfoundation.org>
> Acked-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
>  Documentation/process/submitting-patches.rst | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> index 7b0ac7370cb1..6754bc15f989 100644
> --- a/Documentation/process/submitting-patches.rst
> +++ b/Documentation/process/submitting-patches.rst
> @@ -614,6 +614,10 @@ process nor the requirement to Cc: stable@vger.kernel.org on all stable
>  patch candidates. For more information, please read
>  Documentation/process/stable-kernel-rules.rst.
>  
> +Finally, while providing tags is welcome and typically very appreciated, please
> +note that signers (i.e. submitters and maintainers) may use their discretion in
> +applying offered tags.
> +
>  .. _the_canonical_patch_format:
>  
>  The canonical patch format
> -- 
> 2.48.0
> 

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 3/3] docs: submitting-patches: clarify that signers may use their discretion on tags

[Greg Kroah-Hartman]

On Sun, Jan 12, 2025 at 10:47:02AM -0500, Neal Gompa wrote:
> On Sun, Jan 12, 2025 at 10:30 AM Miguel Ojeda <ojeda@kernel.org> wrote:
> >
> > Tags are really appreciated by maintainers in general, since it means
> > someone is willing to put their name on a commit, be it as a reviewer,
> > tester, etc.
> >
> > However, signers (i.e. submitters carrying tags from previous versions
> > and maintainers applying patches) may need to take or drop tags, on a
> > case-by-case basis, for different reasons.
> >
> > Yet this is not explicitly spelled out in the documentation, thus there
> > may be instances [1] where contributors may feel unwelcome.
> >
> > Thus, to clarify, state this clearly.
> >
> > Link: https://lore.kernel.org/rust-for-linux/CAEg-Je-h4NitWb2ErFGCOqt0KQfXuyKWLhpnNHCdRzZdxi018Q@mail.gmail.com/ [1]
> > Suggested-by: Dan Williams <dan.j.williams@intel.com>
> > Acked-by: Shuah Khan <skhan@linuxfoundation.org>
> > Acked-by: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> > ---
> >  Documentation/process/submitting-patches.rst | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> > index 7b0ac7370cb1..6754bc15f989 100644
> > --- a/Documentation/process/submitting-patches.rst
> > +++ b/Documentation/process/submitting-patches.rst
> > @@ -614,6 +614,10 @@ process nor the requirement to Cc: stable@vger.kernel.org on all stable
> >  patch candidates. For more information, please read
> >  Documentation/process/stable-kernel-rules.rst.
> >
> > +Finally, while providing tags is welcome and typically very appreciated, please
> > +note that signers (i.e. submitters and maintainers) may use their discretion in
> > +applying offered tags.
> > +
> >  .. _the_canonical_patch_format:
> >
> >  The canonical patch format
> > --
> > 2.48.0
> >
> 
> A tag must not be dropped without the tag submitter's authorization.
> Otherwise it doesn't matter what you write here, the submitter *will*
> feel unwelcome.
> 
> It is rude and discouraging to do so without their acceptance of doing so.

I accidentally "drop" them all the time by virtue of them coming in
_after_ the patch is committed to my git tree.  I can't rebase, so they
just don't get applied to the commit, which is fine.  The tag stays
"as-is" in the email thread for forever, so there's no real problem
here.

Also, sometimes, we can't apply a tag from some entities for various
reasons, so please don't think that anyone can force a maintainer to
take things that they aren't allowed to, or want to, take.  But this is
a rare occurance.

thanks,

greg k-h

Re: [PATCH 1/3] docs: submitting-patches: clarify Acked-by and introduce "# Suffix"

[Greg Kroah-Hartman]

On Sun, Jan 12, 2025 at 04:29:44PM +0100, Miguel Ojeda wrote:
> Acked-by is typically used by maintainers. However, sometimes it is
> useful to be able to accept the tag from other stakeholders that may not
> have done a deep technical review or may not be kernel developers. For
> instance:
> 
>   - People with domain knowledge, such as the original author of the
>     code being modified.
> 
>   - Userspace-side reviewers for a kernel uAPI patch, like in DRM --
>     see Documentation/gpu/drm-uapi.rst:
> 
>     > The userspace-side reviewer should also provide an Acked-by on the
>     > kernel uAPI patch indicating that they believe the proposed uAPI
>     > is sound and sufficiently documented and validated for userspace's
>     > consumption.
> 
>   - Key users of a feature, such as in [1].
> 
> Thus clarify that Acked-by may be used by other stakeholders (but most
> commonly by maintainers).
> 
> Since, in these cases, it may be confusing why an Acked-by is/was
> provided, allow and suggest to provide a "# Suffix" explaining it.
> 
> The "# Suffix" for Acked-by is already being used to clarify what part
> of the patch a maintainer is acknowledging, thus also mention "# Suffix"
> in the relevant paragraph.
> 
> Link: https://lore.kernel.org/rust-for-linux/CANiq72m4fea15Z0fFZauz8N2madkBJ0G7Dc094OwoajnXmROOA@mail.gmail.com/ [1]
> Acked-by: Shuah Khan <skhan@linuxfoundation.org>
> Acked-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
>  Documentation/process/submitting-patches.rst | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> index 1518bd57adab..c7a28af235f7 100644
> --- a/Documentation/process/submitting-patches.rst
> +++ b/Documentation/process/submitting-patches.rst
> @@ -463,9 +463,17 @@ If a person was not directly involved in the preparation or handling of a
>  patch but wishes to signify and record their approval of it then they can
>  ask to have an Acked-by: line added to the patch's changelog.
> 
> -Acked-by: is often used by the maintainer of the affected code when that
> +Acked-by: is meant to be used by those responsible for or involved with the
> +affected code in one way or another.  Most commonly, the maintainer when that
>  maintainer neither contributed to nor forwarded the patch.
> 
> +Acked-by: may also be used by other stakeholders, such as people with domain
> +knowledge (e.g. the original author of the code being modified), userspace-side
> +reviewers for a kernel uAPI patch or key users of a feature.  Optionally, in
> +these cases, it can be useful to add a "# Suffix" to clarify its meaning::
> +
> +	Acked-by: The Stakeholder <stakeholder@example.org> # As primary user
> +
>  Acked-by: is not as formal as Signed-off-by:.  It is a record that the acker
>  has at least reviewed the patch and has indicated acceptance.  Hence patch
>  mergers will sometimes manually convert an acker's "yep, looks good to me"
> @@ -477,7 +485,7 @@ For example, if a patch affects multiple subsystems and has an Acked-by: from
>  one subsystem maintainer then this usually indicates acknowledgement of just
>  the part which affects that maintainer's code.  Judgement should be used here.
>  When in doubt people should refer to the original discussion in the mailing
> -list archives.
> +list archives.  A "# Suffix" may also be used in this case to clarify.
> 
>  If a person has had the opportunity to comment on a patch, but has not
>  provided such comments, you may optionally add a ``Cc:`` tag to the patch.
> --
> 2.48.0

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 2/3] docs: submitting-patches: clarify difference between Acked-by and Reviewed-by

[Greg Kroah-Hartman]

On Sun, Jan 12, 2025 at 04:29:45PM +0100, Miguel Ojeda wrote:
> Newcomers to the kernel need to learn the different tags that are
> used in commit messages and when to apply them. Acked-by is sometimes
> misunderstood, since the documentation did not really clarify (up to
> the previous commit) when it should be used, especially compared to
> Reviewed-by.
> 
> The previous commit already clarified who the usual providers of Acked-by
> tags are, with examples. Thus provide a clarification paragraph for
> the comparison with Reviewed-by, and give a couple examples reusing the
> cases given above, in the previous commit.
> 
> Acked-by: Shuah Khan <skhan@linuxfoundation.org>
> Acked-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
>  Documentation/process/submitting-patches.rst | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> index c7a28af235f7..7b0ac7370cb1 100644
> --- a/Documentation/process/submitting-patches.rst
> +++ b/Documentation/process/submitting-patches.rst
> @@ -480,6 +480,12 @@ mergers will sometimes manually convert an acker's "yep, looks good to me"
>  into an Acked-by: (but note that it is usually better to ask for an
>  explicit ack).
>  
> +Acked-by: is also less formal than Reviewed-by:.  For instance, maintainers may
> +use it to signify that they are OK with a patch landing, but they may not have
> +reviewed it as thoroughly as if a Reviewed-by: was provided.  Similarly, a key
> +user may not have carried out a technical review of the patch, yet they may be
> +satisfied with the general approach, the feature or the user-facing interface.
> +
>  Acked-by: does not necessarily indicate acknowledgement of the entire patch.
>  For example, if a patch affects multiple subsystems and has an Acked-by: from
>  one subsystem maintainer then this usually indicates acknowledgement of just
> -- 
> 2.48.0
> 

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 2/3] docs: submitting-patches: clarify difference between Acked-by and Reviewed-by

[Jonathan Corbet]

Neal Gompa <neal@gompa.dev> writes:

> I've had my Reviewed-by tags silently ignored or deliberately stripped
> because even though I've done a technical review, the maintainer does
> not believe that I did. Therefore, what I am saying is that
> maintainers seem to speciously decide whether an Acked-by or
> Reviewed-by tag is appropriate or not *after* someone has sent it.
>
> This is the fundamental problem I have right now. This decision is not
> the maintainer's to make, it is the submitter's.

There *are* people who seem to make a game of getting as many such tags
into the repository as possible.  I think a bit of maintainer discretion
is important here; I don't believe that there is a fundamental right to
inject a tag into somebody else's patch...?

jon

Re: [PATCH 2/3] docs: submitting-patches: clarify difference between Acked-by and Reviewed-by

[Neal Gompa]

On Sun, Jan 12, 2025 at 2:59 PM Jonathan Corbet <corbet@lwn.net> wrote:
>
> Neal Gompa <neal@gompa.dev> writes:
>
> > I've had my Reviewed-by tags silently ignored or deliberately stripped
> > because even though I've done a technical review, the maintainer does
> > not believe that I did. Therefore, what I am saying is that
> > maintainers seem to speciously decide whether an Acked-by or
> > Reviewed-by tag is appropriate or not *after* someone has sent it.
> >
> > This is the fundamental problem I have right now. This decision is not
> > the maintainer's to make, it is the submitter's.
>
> There *are* people who seem to make a game of getting as many such tags
> into the repository as possible.  I think a bit of maintainer discretion
> is important here; I don't believe that there is a fundamental right to
> inject a tag into somebody else's patch...?
>

I think there's a difference between not being collected in review
cycles and being stripped on application/merge to the Git repo. The
latter is something I've experienced before.




--
真実はいつも一つ！/ Always, there's only one truth!

Re: [PATCH 2/3] docs: submitting-patches: clarify difference between Acked-by and Reviewed-by

[Krzysztof Kozlowski]

On 12/01/2025 16:50, Neal Gompa wrote:
> On Sun, Jan 12, 2025 at 10:30 AM Miguel Ojeda <ojeda@kernel.org> wrote:
>>
>> Newcomers to the kernel need to learn the different tags that are
>> used in commit messages and when to apply them. Acked-by is sometimes
>> misunderstood, since the documentation did not really clarify (up to
>> the previous commit) when it should be used, especially compared to
>> Reviewed-by.
>>
>> The previous commit already clarified who the usual providers of Acked-by
>> tags are, with examples. Thus provide a clarification paragraph for
>> the comparison with Reviewed-by, and give a couple examples reusing the
>> cases given above, in the previous commit.
>>
>> Acked-by: Shuah Khan <skhan@linuxfoundation.org>
>> Acked-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
>> ---
>>  Documentation/process/submitting-patches.rst | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
>> index c7a28af235f7..7b0ac7370cb1 100644
>> --- a/Documentation/process/submitting-patches.rst
>> +++ b/Documentation/process/submitting-patches.rst
>> @@ -480,6 +480,12 @@ mergers will sometimes manually convert an acker's "yep, looks good to me"
>>  into an Acked-by: (but note that it is usually better to ask for an
>>  explicit ack).
>>
>> +Acked-by: is also less formal than Reviewed-by:.  For instance, maintainers may
>> +use it to signify that they are OK with a patch landing, but they may not have
>> +reviewed it as thoroughly as if a Reviewed-by: was provided.  Similarly, a key
>> +user may not have carried out a technical review of the patch, yet they may be
>> +satisfied with the general approach, the feature or the user-facing interface.
>> +
>>  Acked-by: does not necessarily indicate acknowledgement of the entire patch.
>>  For example, if a patch affects multiple subsystems and has an Acked-by: from
>>  one subsystem maintainer then this usually indicates acknowledgement of just
>> --
>> 2.48.0
>>
> 
> This doesn't make sense as a distinction. What defines "thoroughly"?

IMO, reviewers statement of oversight. One should not give reviewed-by
tags, if one does not agree fully with the statement.

> To be honest, I think you should go the other way and become okay with
> people sending Reviewed-by tags when people have looked over a patch
> and consider it good to land.

No, that dilutes the review. There always was Ack for such cases.

Best regards,
Krzysztof

Re: [PATCH 2/3] docs: submitting-patches: clarify difference between Acked-by and Reviewed-by

[Krzysztof Kozlowski]

On 12/01/2025 16:29, Miguel Ojeda wrote:
> Newcomers to the kernel need to learn the different tags that are
> used in commit messages and when to apply them. Acked-by is sometimes
> misunderstood, since the documentation did not really clarify (up to
> the previous commit) when it should be used, especially compared to
> Reviewed-by.
> 
> The previous commit already clarified who the usual providers of Acked-by
> tags are, with examples. Thus provide a clarification paragraph for
> the comparison with Reviewed-by, and give a couple examples reusing the
> cases given above, in the previous commit.
> 


Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH 3/3] docs: submitting-patches: clarify that signers may use their discretion on tags

[Krzysztof Kozlowski]

On 12/01/2025 16:29, Miguel Ojeda wrote:
> Tags are really appreciated by maintainers in general, since it means
> someone is willing to put their name on a commit, be it as a reviewer,
> tester, etc.
> 
> However, signers (i.e. submitters carrying tags from previous versions
> and maintainers applying patches) may need to take or drop tags, on a
> case-by-case basis, for different reasons.
> 
> Yet this is not explicitly spelled out in the documentation, thus there
> may be instances [1] where contributors may feel unwelcome.
> 
> Thus, to clarify, state this clearly.
> 
> Link: https://lore.kernel.org/rust-for-linux/CAEg-Je-h4NitWb2ErFGCOqt0KQfXuyKWLhpnNHCdRzZdxi018Q@mail.gmail.com/ [1]
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Acked-by: Shuah Khan <skhan@linuxfoundation.org>
> Acked-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH 1/3] docs: submitting-patches: clarify Acked-by and introduce "# Suffix"

[Krzysztof Kozlowski]

On 12/01/2025 16:29, Miguel Ojeda wrote:
> Acked-by is typically used by maintainers. However, sometimes it is
> useful to be able to accept the tag from other stakeholders that may not
> have done a deep technical review or may not be kernel developers. For
> instance:
> 
>   - People with domain knowledge, such as the original author of the
>     code being modified.
> 
>   - Userspace-side reviewers for a kernel uAPI patch, like in DRM --
>     see Documentation/gpu/drm-uapi.rst:
> 
>     > The userspace-side reviewer should also provide an Acked-by on the
>     > kernel uAPI patch indicating that they believe the proposed uAPI
>     > is sound and sufficiently documented and validated for userspace's
>     > consumption.
> 
>   - Key users of a feature, such as in [1].
> 
> Thus clarify that Acked-by may be used by other stakeholders (but most
> commonly by maintainers).
> 
> Since, in these cases, it may be confusing why an Acked-by is/was
> provided, allow and suggest to provide a "# Suffix" explaining it.
> 
> The "# Suffix" for Acked-by is already being used to clarify what part
> of the patch a maintainer is acknowledging, thus also mention "# Suffix"
> in the relevant paragraph.
> 
> Link: https://lore.kernel.org/rust-for-linux/CANiq72m4fea15Z0fFZauz8N2madkBJ0G7Dc094OwoajnXmROOA@mail.gmail.com/ [1]
> Acked-by: Shuah Khan <skhan@linuxfoundation.org>
> Acked-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
>  Documentation/process/submitting-patches.rst | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)

Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
Best regards,
Krzysztof

Re: [PATCH 2/3] docs: submitting-patches: clarify difference between Acked-by and Reviewed-by

[Jani Nikula]

On Sun, 12 Jan 2025, Neal Gompa <neal@gompa.dev> wrote:
> On Sun, Jan 12, 2025 at 10:30 AM Miguel Ojeda <ojeda@kernel.org> wrote:
>>
>> Newcomers to the kernel need to learn the different tags that are
>> used in commit messages and when to apply them. Acked-by is sometimes
>> misunderstood, since the documentation did not really clarify (up to
>> the previous commit) when it should be used, especially compared to
>> Reviewed-by.
>>
>> The previous commit already clarified who the usual providers of Acked-by
>> tags are, with examples. Thus provide a clarification paragraph for
>> the comparison with Reviewed-by, and give a couple examples reusing the
>> cases given above, in the previous commit.
>>
>> Acked-by: Shuah Khan <skhan@linuxfoundation.org>
>> Acked-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
>> ---
>>  Documentation/process/submitting-patches.rst | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
>> index c7a28af235f7..7b0ac7370cb1 100644
>> --- a/Documentation/process/submitting-patches.rst
>> +++ b/Documentation/process/submitting-patches.rst
>> @@ -480,6 +480,12 @@ mergers will sometimes manually convert an acker's "yep, looks good to me"
>>  into an Acked-by: (but note that it is usually better to ask for an
>>  explicit ack).
>>
>> +Acked-by: is also less formal than Reviewed-by:.  For instance, maintainers may
>> +use it to signify that they are OK with a patch landing, but they may not have
>> +reviewed it as thoroughly as if a Reviewed-by: was provided.  Similarly, a key
>> +user may not have carried out a technical review of the patch, yet they may be
>> +satisfied with the general approach, the feature or the user-facing interface.
>> +
>>  Acked-by: does not necessarily indicate acknowledgement of the entire patch.
>>  For example, if a patch affects multiple subsystems and has an Acked-by: from
>>  one subsystem maintainer then this usually indicates acknowledgement of just
>> --
>> 2.48.0
>>
>
> This doesn't make sense as a distinction. What defines "thoroughly"?
> To be honest, I think you should go the other way and become okay with
> people sending Reviewed-by tags when people have looked over a patch
> and consider it good to land.
>
> To me, Acked-by mostly makes sense as a tag for people who *won't*
> review the code, not for those who *will*. Blending Acked-by and
> Reviewed-by just creates confusion.

As a maintainer, I mostly use Acked-by for two slightly different cases:

1) I've seen the patch. I have no objections to it being merged, I
   approve of it. I haven't done a detailed review of it. Additionally,
   I may indicate whether a detailed review (by someone else) is
   required, or whether I think the ack is sufficient for merging.

2) I'm fine with the patch to the area I maintain being merged via some
   other maintainers' repositories. I may or may not have also given my
   Reviewed-by in this case, which alone is not an approval to merge via
   other trees.

I think this pretty much aligns with the patch series.


BR,
Jani.

-- 
Jani Nikula, Intel

Re: [PATCH 3/3] docs: submitting-patches: clarify that signers may use their discretion on tags

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1049 bytes --]

On Sun, Jan 12, 2025 at 10:47:02AM -0500, Neal Gompa wrote:
> On Sun, Jan 12, 2025 at 10:30 AM Miguel Ojeda <ojeda@kernel.org> wrote:

> > +Finally, while providing tags is welcome and typically very appreciated, please
> > +note that signers (i.e. submitters and maintainers) may use their discretion in
> > +applying offered tags.
> > +

> A tag must not be dropped without the tag submitter's authorization.
> Otherwise it doesn't matter what you write here, the submitter *will*
> feel unwelcome.

> It is rude and discouraging to do so without their acceptance of doing so.

As well as the issues that Miguel and Greg mentioned there's cases where
we specifically want to drop tags, the most obvious one being that if a
patch or possibly even it's context changes substantially between
versions that will invalidate the testing or review that was done on
prior versions.  Perhaps we might want something like "please note that
while the default is to apply tags signers..." but I don't think we'd
want anything stronger.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/3] docs: submitting-patches: clarify difference between Acked-by and Reviewed-by

[Theodore Ts'o]

On Sun, Jan 12, 2025 at 11:35:29AM -0500, Neal Gompa wrote:
> I've had my Reviewed-by tags silently ignored or deliberately stripped
> because even though I've done a technical review, the maintainer does
> not believe that I did. Therefore, what I am saying is that
> maintainers seem to speciously decide whether an Acked-by or
> Reviewed-by tag is appropriate or not *after* someone has sent it.
> 
> This is the fundamental problem I have right now. This decision is not
> the maintainer's to make, it is the submitter's.

Nope, it's is *absolutely* the maintainer's to make.  If I don't trust
the reviewer, or reviewer is known to be someone who is only giving
English grammer or policy nit-picking, I am absolutely going to ignore
the reviewer, and in some cases, tell the patch submitter that should
feel to ignore the reviewer or feel that they should be obliged to pay
attention to certain reviewers.

Note also that a submitter is not obliged to resend a patch if someone
sends an Acked-by or Reviewed-by.  There are tools like b4 or
patchwork that will blindly add all tags, and then it's up to the
maintainer to remove any b.s. tags that maintainer doesn't feel up to
the subsystem's standards.

						- Ted

Re: [PATCH 3/3] docs: submitting-patches: clarify that signers may use their discretion on tags

[Theodore Ts'o]

On Sun, Jan 12, 2025 at 10:47:02AM -0500, Neal Gompa wrote:
> 
> A tag must not be dropped without the tag submitter's authorization.
> Otherwise it doesn't matter what you write here, the submitter *will*
> feel unwelcome.
> 
> It is rude and discouraging to do so without their acceptance of doing so.

In some cases, if the reviewer hasn't taken the less than subtle hints
that their reviews are unwelcome and are discouraging patch
submitters, as far as I'm concerned, if they feel unwelcome, that is a
*feature* and not a *bug*.

I'm not saying that is the case for you, but there are reviewers that
add negative value in the ecosystem.  The assumption that all tag
submitters are people that need to feel welcome might mostly true, but
it is not always the case.  This is why it MUST ultimately be up to
the maintainer.  I do not want the rules that force me to reward
people that should be discouraged, not encouraged.

						- Ted

Re: [PATCH 2/3] docs: submitting-patches: clarify difference between Acked-by and Reviewed-by

[Steven Rostedt]

On Mon, 13 Jan 2025 14:38:00 +0200
Jani Nikula <jani.nikula@intel.com> wrote:

> As a maintainer, I mostly use Acked-by for two slightly different cases:
> 
> 1) I've seen the patch. I have no objections to it being merged, I
>    approve of it. I haven't done a detailed review of it. Additionally,
>    I may indicate whether a detailed review (by someone else) is
>    required, or whether I think the ack is sufficient for merging.
> 
> 2) I'm fine with the patch to the area I maintain being merged via some
>    other maintainers' repositories. I may or may not have also given my
>    Reviewed-by in this case, which alone is not an approval to merge via
>    other trees.

Interesting. When I give a Reviewed-by: to a patch, I am most definitely
letting it be merged into other trees. For anything I pull in, I don't add
a Reviewed-by and will strip any tag that says I did review it as my
Signed-off-by includes that I reviewed the patch.

The difference I give between Acked-by and Reviewed-by is that my Acked-by
is "I don't see anything wrong with the idea of the change, and it can go
via another tree", where as a Reviewed-by is "I took time to understand the
change itself, and have not found anything wrong with it".

Basically, an Acked-by is "I took a quick look, and I'm OK with it, but if
it breaks something of mine, I expect you to fix it." and Reviewed-by is "I
took a deeper look, and if it breaks something of mine, I hold myself at
fault, and will fix it myself". ;-)

-- Steve

Re: [PATCH 3/3] docs: submitting-patches: clarify that signers may use their discretion on tags

[Steven Rostedt]

On Mon, 13 Jan 2025 09:22:27 -0500
"Theodore Ts'o" <tytso@mit.edu> wrote:

> On Sun, Jan 12, 2025 at 10:47:02AM -0500, Neal Gompa wrote:
> > 
> > A tag must not be dropped without the tag submitter's authorization.
> > Otherwise it doesn't matter what you write here, the submitter *will*
> > feel unwelcome.
> > 
> > It is rude and discouraging to do so without their acceptance of doing so.  
> 
> In some cases, if the reviewer hasn't taken the less than subtle hints
> that their reviews are unwelcome and are discouraging patch
> submitters, as far as I'm concerned, if they feel unwelcome, that is a
> *feature* and not a *bug*.
> 
> I'm not saying that is the case for you, but there are reviewers that
> add negative value in the ecosystem.  The assumption that all tag
> submitters are people that need to feel welcome might mostly true, but
> it is not always the case.  This is why it MUST ultimately be up to
> the maintainer.  I do not want the rules that force me to reward
> people that should be discouraged, not encouraged.

I try to add all tags that are meaningful, but a lot of times I already
pulled in the patch, and there's been several times tags came in after I
did a pull request to Linus! (for fixes patches). Once I do a pull request to
Linus, only Linus can have me change that commit (if there's an issue with
it).

For linux-next, I will add tags, even when Linus told me to avoid doing so.

That said, I have ignored several "Reviewed-by" tags when they appear
random. If you send a "Reviewed-by" without a single comment, and I don't
know you, it is meaningless to me, and I will not add it.

Now for people I don't know, if they came in and found an issue with a
patch which requires another revision, I will most definitely add their
Reviewed-by for the next version (if they send one), and will likely take
Reviewed-by tags from them in the future without comments (up to a point).

There's also been several cases that someone sent a Reviewed-by tag and
when I take a look at the patch I find an obvious bug. That will cause me
to ignore that reviewer in the future. Adding that tag comes with
responsibilities. As I mentioned in another thread, if I give a Reviewed-by
and a bug happens on that commit, I will spend time to help fix it.

-- Steve

Re: [PATCH 0/3] Clarifications around Acked-by and "# Suffix" proposal

[Jonathan Corbet]

Miguel Ojeda <ojeda@kernel.org> writes:

> A patch series that aims to clarify what the purpose of the Acked-by tag is,
> in order to help newcomers understand the kernel process better, as well as
> to give extra flexibility on the usage of the Acked-by tag.
>
> It formalizes one of the proposals from [1]: "# Suffix" for Acked-by.
>
> I hope this helps!
>
> Link: https://lore.kernel.org/rust-for-linux/CANiq72m4fea15Z0fFZauz8N2madkBJ0G7Dc094OwoajnXmROOA@mail.gmail.com/ [1]
>
> Miguel Ojeda (3):
>   docs: submitting-patches: clarify Acked-by and introduce "# Suffix"
>   docs: submitting-patches: clarify difference between Acked-by and
>     Reviewed-by
>   docs: submitting-patches: clarify that signers may use their
>     discretion on tags
>
>  Documentation/process/submitting-patches.rst | 22 ++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)

I've applied the set, thanks.

jon

Re: [PATCH 2/3] docs: submitting-patches: clarify difference between Acked-by and Reviewed-by

[Darrick J. Wong]

On Sun, Jan 12, 2025 at 10:50:32AM -0500, Neal Gompa wrote:
> On Sun, Jan 12, 2025 at 10:30 AM Miguel Ojeda <ojeda@kernel.org> wrote:
> >
> > Newcomers to the kernel need to learn the different tags that are
> > used in commit messages and when to apply them. Acked-by is sometimes
> > misunderstood, since the documentation did not really clarify (up to
> > the previous commit) when it should be used, especially compared to
> > Reviewed-by.
> >
> > The previous commit already clarified who the usual providers of Acked-by
> > tags are, with examples. Thus provide a clarification paragraph for
> > the comparison with Reviewed-by, and give a couple examples reusing the
> > cases given above, in the previous commit.
> >
> > Acked-by: Shuah Khan <skhan@linuxfoundation.org>
> > Acked-by: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> > ---
> >  Documentation/process/submitting-patches.rst | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> > index c7a28af235f7..7b0ac7370cb1 100644
> > --- a/Documentation/process/submitting-patches.rst
> > +++ b/Documentation/process/submitting-patches.rst
> > @@ -480,6 +480,12 @@ mergers will sometimes manually convert an acker's "yep, looks good to me"
> >  into an Acked-by: (but note that it is usually better to ask for an
> >  explicit ack).
> >
> > +Acked-by: is also less formal than Reviewed-by:.  For instance, maintainers may
> > +use it to signify that they are OK with a patch landing, but they may not have
> > +reviewed it as thoroughly as if a Reviewed-by: was provided.  Similarly, a key
> > +user may not have carried out a technical review of the patch, yet they may be
> > +satisfied with the general approach, the feature or the user-facing interface.
> > +
> >  Acked-by: does not necessarily indicate acknowledgement of the entire patch.
> >  For example, if a patch affects multiple subsystems and has an Acked-by: from
> >  one subsystem maintainer then this usually indicates acknowledgement of just
> > --
> > 2.48.0
> >
> 
> This doesn't make sense as a distinction. What defines "thoroughly"?
> To be honest, I think you should go the other way and become okay with
> people sending Reviewed-by tags when people have looked over a patch
> and consider it good to land.
> 
> To me, Acked-by mostly makes sense as a tag for people who *won't*
> review the code, not for those who *will*. Blending Acked-by and
> Reviewed-by just creates confusion.

Not a maintainer anymore, but --

I only give out a Reviewed-by: if I can say with a straight face "I read
this code thoroughly and understand it well enough to transform / build
on top of / maintain it if need be." I'd accept one from anyone who I
thought was either really familiar with the codebase or has become their
manager's stuc^Wappointee for maintenance.

Compare that to an Acked-by, which means "I scanned this while
doomscrolling fsdevel over coffee and none of it is now in the
keyboard", which is a much lower standard.  I'd accept one from pretty
much anyone, because that just means you're in the email blasting radius
if/when things go wrong.  Even moreso if the person qualifies their ack
with a "# XXXX" to contextualize their acknowledgement.

Concretely, I might ignore an RVB from Sam Naghshineh if he showed up
claiming to be an expert on some ext4 thing, but I wouldn't drop an Ack
from Neal because then who do I pull in when boffins demonstrate that
fallocate implements a Turing machine and hence in need of a libvirt
port?

I would, however, explicitly point out that maintainers can drop or
ignore tags as they please; and that doing so may discourage future
participation by people who feel ignored.

--D

> 
> 
> -- 
> 真実はいつも一つ！/ Always, there's only one truth!