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 15:14, Eli Zaretskii wrote:
>> Date: Mon, 10 Oct 2022 14:31:54 +0200
>> Cc: [email protected], [email protected]
>> From: Bruno Larsen <[email protected]>
>>
>> On 10/10/2022 13:27, Eli Zaretskii wrote:
>>>> Date: Mon, 10 Oct 2022 12:11:46 +0200
>>>> Cc: [email protected], [email protected]
>>>> From: Bruno Larsen <[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".
>>> I don't mind either way.  This whole thing is a service to others, so
>>> I'll do whatever people prefer.  Let me just point out that my
>>> situation is not too unique: several other maintainers can approve
>>> only parts of patches.
>> Ah, so I'll suggest that you approve the documentation changes, and I'll
>> mention that some approvers may sometimes only approve part of the
>> patch, so one should make sure the whole patch is approved before pushing.
> I'm not sure I understand: do you mean that I should not use _any_ tag
> at all, when the patch includes more than just documentation?

Sorry for making things even more confusing. I meant that you use 
Approved-by tags.

What I was getting at was that the wording I have been using until this 
point was "if you get any Approved-by, it is ready for pushing", and 
that needs to be changed to "be sure that you have received Approved-by 
for all the code, in case a maintainer only approved parts of your 
change", or something of the sorts. Still working on that...

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.