Re: [PATCH v2] Documentation: add ReviewingGuidelines

From: Elijah Newren <newren@gmail.com>
Date: Sep 20, 2022, 00:43 UTC
Message-ID: <CABPp-BEB_+YoKZ=U6NPc8J+KZyMSYRsom34CeqjxUCyw0=LEyg@mail.gmail.com>
In-Reply-To: <pull.1348.v2.git.1663614767058.gitgitgadget@gmail.com>

On Mon, Sep 19, 2022 at 12:21 PM Victoria Dye via GitGitGadget <gitgitgadget@gmail.com> wrote:

[...]

Show 10 quoted lines

> +==== 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=.

[...]

Show 5 quoted lines

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