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