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

[Bruno Larsen <blarsen@redhat.com>]

On 16/05/2023 18:04, Eli Zaretskii wrote:
>> Cc: gdb@sourceware.org,
>> 	Bruno Larsen <blarsen@redhat.com>
>> Date: Tue, 16 May 2023 16:38:27 +0200
>> From: Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org>
>>
>> + - Tested-by:
>> +
>> +   Used when a contributor does not want to comment on the quality
>> +   of the code in the patch, but has tested and sees no regressions on their
>> +   hardware.
>> +
>> + - Reviewed-by:
>> +
>> +   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 tried using "contributor" to mean 
anyone who decided to look at a patch, so it would include responsible 
maintainers when they dont have authority (or desire) to approve a 
patch. How do you think it could be rephrased to make it more clear?
>
>> +
>> + - Approved-by:
>> +
>> +   Used by responsible mainainers or global maintainers when
>                            ^^^^^^^^^^
> Typo.
oops sorry
>
>> +   a patch is ready to be upstreamed.  Some patches may touch multiple areas
>> +   and require multiple approvals before landing (such as a maintainer only
>> +   approving documentation), it is up to the maintainer giving the approval tag
>> +   to make it clear when that a tag is not sufficient.
>> +   Responsible, Global and Official FSF-appointed maintainers may approve their
>> +   own patches, but it is recommended that they seek external approval before
>> +   doing so.
>> +
> 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 :-)

As to why call them "git trailers", I copied over from the checklist[1], 
that was already there describing Co-Authored-By and Bug trailers. We 
can change the name to just trailers or tags or something else if you 
prefer.

[1] 
https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_formatted_commit_messages

-- 
Cheers,
Bruno