Re: Proposal: Add review tags to patch review workflow.

Bruno Larsen via Gdb <[email protected]>
Newsgroups gmane.comp.gdb.devel
Message-ID <[email protected]>
On 10/10/2022 11:47, Eli Zaretskii wrote:
>> Date: Mon, 10 Oct 2022 11:27:03 +0200
>> Cc: [email protected]
>> From: Bruno Larsen <[email protected]>
>>
>> As Simon mentioned, there weren't big changes, but here's a quick cookbook:
> Thanks!
>
>> 1. If you have the authority to approve a patch and believe the patch
>> you are reviewing is ready to be merged, add the following line to your
>> e-mail (usually at the end): Approved-by: Your Name <[email protected]>
>>
>> 2. If you don't have the authority to approve patches, or aren't
>> convinced that you know enough about the area of code to fully approve a
>> patch for merging, and haven't found any technical issues (i.e.
>> non-nitpicks) with the patch, add the following line to your e-mail:
>> Reviewed-by: Your Name <[email protected]>
>>
>> 3. If you aren't sure of the quality of the technical changes, but you
>> have tested and verified that the patch does not add any regressions,
>> add the following line to your e-mail: Tested-by: Your Name
>> <[email protected]>
> I'm not clear what I should do when I approve just part of a patch.
> It is frequently the case that a patch includes both code and
> documentation, and I'm approving just the documentation part(s).  Is
> that item 1 or item 2? or something else?
>
It's a bit up to you, if I'm honest. I would default to telling you to 
use Reviewed-by, to avoid confusion, but if you want to say that the 
"documentation parts are Approved-by", I am fine with it.

Just let me know if you decide to go with the second, so I can mention 
in the wiki something like "make sure all of your patch is approved 
before pushing".

Cheers,
Bruno
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.