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

[Juergen Gross <jgross@suse.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2691 bytes --]

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.


Juergen

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]