Re: [PATCH v2] Documentation: add ReviewingGuidelines

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/19/2022 5:43 PM, Elijah Newren wrote:
> On Mon, Sep 19, 2022 at 12:21 PM Victoria Dye via GitGitGadget
> <gitgitgadget@xxxxxxxxx> 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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux