Re: [PATCH v2] Documentation: add ReviewingGuidelines

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

 



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.)

> +- Large, complicated patch diffs are sometimes unavoidable, such as when they
> +  refactor existing code. If you find such a patch difficult to parse, try
> +  reviewing the diff produced with the `--color-moved` and/or
> +  `--ignore-space-change` options.

Similarly, Documentation refactorings or significant rewordings are
sometimes easier to view with --color-words or --color-words=.

[...]
> +See Also
> +--------
> +link:MyFirstContribution.html[MyFirstContribution]
>
> base-commit: 79f2338b3746d23454308648b2491e5beba4beff

I like this document!  I had a couple ideas that might or might not
make sense to include in the document; it looks good to me either way.



[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