Re: [PATCH] MyFirstContribution: Document --range-diff option when writing v2

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sep 13, 2021, 20:00 UTC
Message-ID: <CAPig+cRZaonjCKt-rKcU0y-xNX1OQyftuEkuB4Rzhc5jCwVgMQ@mail.gmail.com>
In-Reply-To: <20210913194816.51182-1-chooglen@google.com>

On Mon, Sep 13, 2021 at 3:48 PM Glen Choo <chooglen@google.com> wrote:

Show 7 quoted lines

> In the "Sending V2" section, readers are directed to create v2 patches
> without using --range-diff. However, it is custom to include a range
> diff against the v1 patches as a reviewer aid.
>
> Update the "Sending V2" section to include the --range-diff option. Also
> include some explanation for -v2 and --range-diff to help the reader
> understand the importance.

Makes sense. A few minor comments below...

Show 13 quoted lines

> Signed-off-by: Glen Choo <chooglen@google.com>
> ---
> diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
> @@ -1033,18 +1033,33 @@ Skip ahead to <<reviewing,Responding to Reviews>> for information on how to
> -When you're ready with the next iteration of your patch, the process is fairly
> -similar.
> +Let's write v2 as its own topic branch, because this will make some things more
> +convenient later on. Create the `psuh-v2` branch like so:
>
> -First, generate your v2 patches again:
> +----
> +$ git checkout -b psuh-v2 psuh
> +----

These days, we're generally promote `git switch -c psuh-v2 psuh` rather than `git branch -b`. However, since the document already uses `git branch -b` elsewhere, the use here is probably acceptable. (An alternative would be to make this a two-patch series in which the first patch changes `git branch -b` over to `git switch -c`.)

Show 7 quoted lines

> +When you're ready with the next iteration of your patch, the process is fairly
> +similar to before. Generate your patches again, but with some new flags:
>
>  ----
> -$ git format-patch -v2 --cover-letter -o psuh/ master..psuh
> +$ git format-patch -v2 --range-diff psuh..psuh-v2 --cover-letter -o psuh/ master..psuh
>  ----

As long as both versions have the same base, it's generally easier to say merely `--range-diff=psuh` -- that is, you want a range-diff against `psuh` -- than to spell out the range explicitly. However, perhaps spelling out the range here has some pedagogical value, so maybe this is okay as-is.

> +The `--range-diff psuh..psuh-v2` parameter tells `format-patch` to include a
> +range diff between `psuh` and `psuh-v2`. This helps tell reviewers about the
> +differences between your v1 and v2 patches.

I think we usually spell it as "range-diff", not "range diff". Also, it might be a good idea to give some hint as to what a range-diff is, even if that hint is just a link to the `git range-diff` manual page. Maybe:

    ...to include a range-diff (see linkgit:range-diff[1]) between...