Re: [PATCH v2] docs: submitting-patches: Clarify half-baked review tags
From: Krzysztof Kozlowski
Date: Thu Nov 05 2026 - 10:46:22 EST
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.
Best regards,
Krzysztof