Re: [PATCH v2] Documentation: add ReviewingGuidelines

From: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
Date: Sep 22, 2022, 04:24 UTC
Message-ID: <df9b1022-fc96-f0fe-8652-78e8891e3c99@gmail.com>
In-Reply-To: <CABPp-BEB_+YoKZ=U6NPc8J+KZyMSYRsom34CeqjxUCyw0=LEyg@mail.gmail.com>

On 9/19/2022 5:43 PM, Elijah Newren wrote:

Show 25 quoted lines

> On Mon, Sep 19, 2022 at 12:21 PM Victoria Dye via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>>
> [...]
>> +==== Performing your review
>> +- Provide your review comments per-patch in a plaintext "Reply-All" email to the
>> +  relevant patch. Comments should be made inline, immediately below the relevant
>> +  section(s).
>> +
>> +- You may find that the limited context provided in the patch diff is sometimes
>> +  insufficient for a thorough review. In such cases, you can review patches in
>> +  your local tree by either applying patches with linkgit:git-am[1] or checking
>> +  out the associated branch from https://github.com/gitster/git once the series
>> +  is tracked there.
> 
> Lots of reviews also come with "Fetch-It-Via" instructions in the
> cover letter, making it really easy to grab.  Might be worth
> mentioning?
> 
> Also, would it make sense for us to replace "applying" with
> "downloading and applying", perhaps mentioning `b4 am` for the
> downloading half?
> 
> (I tend to use the Fetch-It-Via or wait for it to show up in
> gitster/git, but b4 is really nice for the other cases.)

Thanks, I did not know about b4, it looks quite helpful!

I think it is worth mentioning some recommended practices to operate 'git-am'. 'git-am' was a bit confusing the first time I tried to grab people's patches from the mailing list without using "Fetch-It-Via", e.g. what is mailbox and how to convert emails into mailbox.

b4 sounds like a good start to add these practices, and probably some other recommendations (I don't know much here)?

Please note that these are just some thoughts, the document itself looks good without adding these practices (maybe we can add them later) :-)

[...]

Thanks, Shaoxuan