[PATCH] docs: submitting-patches: Clarify half-baked review tags

[PATCH] docs: submitting-patches: Clarify half-baked review tags

[Krzysztof Kozlowski]

Refactoring of driver subsystems or some core driver parts often come
with change of the actual drivers.  If that happens within one patch,
people giving "Reviewed-by" tag only to piece of the patch, like one
driver, and ignoring entire rest including the main concept of the
patch, are not really reviewing "the patch".

As explained in Submitting Patches document, the "Reviewer's statement
of oversight" is for patch:

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

The sentence is complete, meaning it does not allow implicit variations
"this patch or part of this patch".

Reviewing pieces of patch while ignoring the main part of it, is
much closer to Acknowledgment, thus Ack, and "Reviewed-by" credit should
not be applicable.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

---

And let me even bring a better example. Imagine a patch, doing massive
refactoring of core SPI stuff, 1000 lines of code, complex locking and
also simultaneously updates drivers to the new API changed in here.

Reviewing that 1000 lines of code in core SPI with complex locking takes
someone 30 minutes. He gives Rb tag, gets credit.

Now another person comes, looks at a SINGLE line in one driver he is
responsible for and gives Rb tag. For 10 seconds job of looking at one
changed call in one driver.
---
 Documentation/process/submitting-patches.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
index f271d7413f72..d2aa31bdee1e 100644
--- a/Documentation/process/submitting-patches.rst
+++ b/Documentation/process/submitting-patches.rst
@@ -585,6 +585,8 @@ serves to give credit to reviewers and to inform maintainers of the degree of
 review which has been done on the patch.  Reviewed-by: tags, when supplied by
 reviewers known to understand the subject area and to perform thorough reviews,
 will normally increase the likelihood of your patch getting into the kernel.
+If the reviewer checks only piece of the patch, leaving the rest unreviewed,
+then more appropriately is Acked-by tag.
 
 Both Tested-by and Reviewed-by tags, once received on mailing list from tester
 or reviewer, should be added by author to the applicable patches when sending
-- 
2.53.0

Re: [PATCH] docs: submitting-patches: Clarify half-baked review tags

[Weijie Yuan]

Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com> writes:

> Refactoring of driver subsystems or some core driver parts often come
> with change of the actual drivers.  If that happens within one patch,
> people giving "Reviewed-by" tag only to piece of the patch, like one
> driver, and ignoring entire rest including the main concept of the
> patch, are not really reviewing "the patch".
> 
> As explained in Submitting Patches document, the "Reviewer's statement
> of oversight" is for patch:
> 
>   (a) I have carried out a technical review of this patch to
>       evaluate its appropriateness and readiness for inclusion into
>       the mainline kernel.
> 
> The sentence is complete, meaning it does not allow implicit variations
> "this patch or part of this patch".

Agreed.

> Reviewing pieces of patch while ignoring the main part of it, is
> much closer to Acknowledgment, thus Ack, and "Reviewed-by" credit should
> not be applicable.

But in this case, I suspect that your "Acked-by" trailer will still be
interpreted as an ack of the entire patch in future git logs, rather
than a partial ack and/or a partial review.

IMO, it would be better to clearly specify which specific parts I
reviewed, like "Reviewed-by: foo # for part a". But if there were no
such usage, I would actually think it would be better not to give
any type of trailers.

> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> 
> ---
> 
> And let me even bring a better example. Imagine a patch, doing massive
> refactoring of core SPI stuff, 1000 lines of code, complex locking and
> also simultaneously updates drivers to the new API changed in here.
> 
> Reviewing that 1000 lines of code in core SPI with complex locking takes
> someone 30 minutes. He gives Rb tag, gets credit.
> 
> Now another person comes, looks at a SINGLE line in one driver he is
> responsible for and gives Rb tag. For 10 seconds job of looking at one
> changed call in one driver.

Yup, therefore, we better suggest that the latter should not give out
any trailers, or explicitly state which parts I have reviewed.

That said, my understanding of these is not as profound as yours.

Thanks.

> ---
>  Documentation/process/submitting-patches.rst | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/process/submitting-patches.rst b/Documentation/process/submitting-patches.rst
> index f271d7413f72..d2aa31bdee1e 100644
> --- a/Documentation/process/submitting-patches.rst
> +++ b/Documentation/process/submitting-patches.rst
> @@ -585,6 +585,8 @@ serves to give credit to reviewers and to inform maintainers of the degree of
>  review which has been done on the patch.  Reviewed-by: tags, when supplied by
>  reviewers known to understand the subject area and to perform thorough reviews,
>  will normally increase the likelihood of your patch getting into the kernel.
> +If the reviewer checks only piece of the patch, leaving the rest unreviewed,
> +then more appropriately is Acked-by tag.
>  
>  Both Tested-by and Reviewed-by tags, once received on mailing list from tester
>  or reviewer, should be added by author to the applicable patches when sending
> -- 
> 2.53.0
> 
> 
> 

Re: [PATCH] docs: submitting-patches: Clarify half-baked review tags

[Krzysztof Kozlowski]

On 05/11/2026 09:34, Weijie Yuan wrote:
> Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com> writes:
> 
>> Refactoring of driver subsystems or some core driver parts often come
>> with change of the actual drivers.  If that happens within one patch,
>> people giving "Reviewed-by" tag only to piece of the patch, like one
>> driver, and ignoring entire rest including the main concept of the
>> patch, are not really reviewing "the patch".
>>
>> As explained in Submitting Patches document, the "Reviewer's statement
>> of oversight" is for patch:
>>
>>   (a) I have carried out a technical review of this patch to
>>       evaluate its appropriateness and readiness for inclusion into
>>       the mainline kernel.
>>
>> The sentence is complete, meaning it does not allow implicit variations
>> "this patch or part of this patch".
> 
> Agreed.
> 
>> Reviewing pieces of patch while ignoring the main part of it, is
>> much closer to Acknowledgment, thus Ack, and "Reviewed-by" credit should
>> not be applicable.
> 
> But in this case, I suspect that your "Acked-by" trailer will still be
> interpreted as an ack of the entire patch in future git logs, rather
> than a partial ack and/or a partial review.
> 

Which would be fine, Acked-by has that meaning and no one really counts
Acks or tracks Acks for any metrics or patch quality purposes.

Best regards,
Krzysztof

Re: [PATCH] docs: submitting-patches: Clarify half-baked review tags

[Weijie Yuan]

Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com> writes:

> On 05/11/2026 09:34, Weijie Yuan wrote:
> > Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com> writes:
> > 
> >> Refactoring of driver subsystems or some core driver parts often come
> >> with change of the actual drivers.  If that happens within one patch,
> >> people giving "Reviewed-by" tag only to piece of the patch, like one
> >> driver, and ignoring entire rest including the main concept of the
> >> patch, are not really reviewing "the patch".
> >>
> >> As explained in Submitting Patches document, the "Reviewer's statement
> >> of oversight" is for patch:
> >>
> >>   (a) I have carried out a technical review of this patch to
> >>       evaluate its appropriateness and readiness for inclusion into
> >>       the mainline kernel.
> >>
> >> The sentence is complete, meaning it does not allow implicit variations
> >> "this patch or part of this patch".
> > 
> > Agreed.
> > 
> >> Reviewing pieces of patch while ignoring the main part of it, is
> >> much closer to Acknowledgment, thus Ack, and "Reviewed-by" credit should
> >> not be applicable.
> > 
> > But in this case, I suspect that your "Acked-by" trailer will still be
> > interpreted as an ack of the entire patch in future git logs, rather
> > than a partial ack and/or a partial review.
> > 
> 
> Which would be fine, Acked-by has that meaning and no one really counts
> Acks or tracks Acks for any metrics or patch quality purposes.

;-) Also makes sense.

(I just posted my last one and found that v2 was also posted, what a
coincidence.)

Thanks!

Re: [PATCH] docs: submitting-patches: Clarify half-baked review tags

[Markus Elfring]

…
> +++ b/Documentation/process/submitting-patches.rst
> @@ -585,6 +585,8 @@ serves to give credit to reviewers and to inform maintainers of the degree of
>  review which has been done on the patch.  Reviewed-by: tags, when supplied by
>  reviewers known to understand the subject area and to perform thorough reviews,
>  will normally increase the likelihood of your patch getting into the kernel.
> +If the reviewer checks only piece of the patch, leaving the rest unreviewed,
> +then more appropriately is Acked-by tag.
…

Possible adjustment:
+an Acked-by tag is more appropriate then.

Regards,
Markus