Re: [PATCH 1/1] [gdb]: add git trailer information on gdb/MAINTAINERS

Simon Marchi via Gdb-patches <[email protected]>
Newsgroups gmane.comp.gdb.patches,gmane.comp.gdb.devel
Message-ID <[email protected]>
On 5/16/23 13:48, Eli Zaretskii via Gdb-patches wrote:
>> Date: Tue, 16 May 2023 18:41:55 +0200
>> Cc: [email protected], [email protected]
>> From: Bruno Larsen <[email protected]>
>>
>> On 16/05/2023 18:04, Eli Zaretskii wrote:
>>>> +   Used when a contributor has looked at code and agrees with the changes,
>>>> +   but either does not have the authority or doesn't feel comfortable
>>>> +   approving the patch (usually due to unfamiliarity with a certain
>>>> +   part of the code).
>>> Reviewed-by is used by responsible maintainers as well.
>> I think I need clearer wording then.
> 
> I think "both contributors and maintainers" is good enough.
> 
>>> I think the above list is incomplete, because there appears to be no
>>> "git trailer" (why do we have to call it "git" trailer, btw?  will
>>> that change if we ever switch to a different VCS?) for the situation
>>> where the responsible maintainer does approve some part of the patch,
>>> but not all of it (e.g., because the other parts are not in the
>>> expertise domain of that maintainer).  I thought Reviewed-by is such a
>>> trailer, but based on the above I'm beginning to think I was confused.
>>>
>> I wrote the proposal based on how I think the use of trailers works on 
>> the QEMU project (I wasn't in it long enough to be sure that I am 
>> correct, though). My thinking was that you'd send something like 
>> "documentation changes are approved, but someone needs to look at the 
>> code, Approved-By ..." or something similar. That said, I just 
>> remembered that they also use Ack-By in those situations and the 
>> maintainer of the subsystem most affected by a change is the only one to 
>> approve the patch, and other relevant maintainers use Ack-By (they have 
>> a very different development workflow, with each subsystem maintainer 
>> having their own tree and them only being merged into the master tree 
>> periodically). I'm pretty open to suggestions, if you think using 
>> Acked-By or some other trailer is better. That is the reason I'm doing 
>> this :-)
> 
> I don't think I'm in a position to put forward suggestions, since I'm
> not sure I have a good understanding of the process.  I only use
> Approved-By when I can approve the entire patch, not just parts of it.
> But maybe I'm wrong in that.

If this happens, I think it's fine to say "the documentation parts are
approved" and following with your Approved-By.  If you want to be
extra-clear, add "but the rest needs to be approved by someone else".
The patch will end up with multiple Approved-Bys.

Speaking of Acked-By, I felt the need to use it recently, where I just
read the commit message, agreed with it, but didn't have time to review
the code itself.  I wanted to show that I agreed with the intent of the
patch.  I think that's what Acked-By is for.  I think we could add it to
that list.

Simon
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.