Re: [PATCH v2] docs: submitting-patches: Clarify half-baked review tags
From: Krzysztof Kozlowski
Date: Thu Nov 05 2026 - 11:13:37 EST
On 05/11/2026 16:44, Juergen Gross wrote:
> On 05.11.26 16:33, Krzysztof Kozlowski wrote:
>> On 05/11/2026 10:30, Jürgen Groß wrote:
>>> On 05.11.26 10:19, Andy Shevchenko wrote:
>>>> On Thu, Nov 05, 2026 at 09:27:30AM +0100, Krzysztof Kozlowski wrote:
>>>>> 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 the rest including the main concept of the patch,
>>>>> are not really reviewing "the patch".
>>>>>
>>>>> As explained in the Submitting Patches document, the "Reviewer's
>>>>> statement of oversight" is for patch as in "entire patch" meaning:
>>>>>
>>>>> (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 explicit and complete, meaning it does not allow
>>>>> implicit variations like: "this patch or part of this patch".
>>>>>
>>>>> Reviewing pieces of the patch while ignoring the main part of it, is
>>>>> much closer to Acknowledgment, thus Ack, and "Reviewed-by" credit should
>>>>> not be applicable.
>>>>
>>>> I strongly disagree. It's all explained in the plenty threads you have started
>>>> on the Subject.
>>>>
>>>> TL;DR: As a reviewer for the piece in question I have done my part fully. It's
>>>> not my issue that the piece is bundled into a single patch. We have options:
>>>> 1) ask to split (and review as standalone piece);
>>>> 2) add (partial) Rb with the pointers of what exactly has been reviewed;
>>>> 3) ignore (silent agreement with the respective subsystem maintainer's opinion);
>>>> 4) ...something I forgot?..
>>>>
>>>> (even partial) Rb != Ack.
>>>>
>>>
>>> I fully agree with Andy.
>>>
>>> As the Xen maintainer I often have to look into such rework patches and often
>>> enough especially the Xen specific parts are mentioned in the patch description
>>> as "compile tested only".
>>>
>>> I see a "Reviewed-by: ... # Xen parts" in such cases as a feedback to the author
>>> or committer that they don't have to look into those parts in more detail.
>>>
>>> The value of such a R-b: is more questionable in case of a pure rename of a
>>> function or struct member, of course.
>>>
>>
>> So you don't agree with "Reviewed-by: # subpart"? Sorry, but I have
>> troubles pointing your disagreement arguments to actual patch content.
> I agree that "Reviewed-by: # subpart" is very valuable and should be used
> in cases where the review really adds some value.
>
> It is different to "Acked-by:" and IMHO not interchangeable with it.
The patch does not make them interchangeable. The patch suggest two
choices if the reviewer's statement of oversight does not apply.
So repeating - which part of patch you disagree with? That reviewer's
statement of oversight says something else?
Best regards,
Krzysztof