Re* [PATCH v3] Documentation: specify base point when generating MyFirstContribution patchset

Commit Message

Junio C Hamano Oct. 18, 2021, 8:08 p.m. UTC

Glen Choo <[e-mail]> writes:

>> But that does not mean master would always be the right base, does
>> it?  What if you had a work totally unrelated to the contents of
>> this tutorial on the 'master' front?  The person on the receiving
>> end may not even know what object it refers to until you pushed your
>> 'master' branch out.
>
> This is the crux of the problem, which is that the contributor's base is
> actually their private 'master' branch, but their patches go to the
> upstream 'master' branch. Does it even matter that the patches were
> originally authored on a private 'master'?

No, and that is why I suggested to update the example to take
advantage of the fact that we forked out of origin/master, not
private master, in my review of the second round.

And telling what the value of origin/master is, or letting
--base=auto to compute the merge base between origin/master and the
branch you are sending out (as origin/master may have advanced since
you forked), which is even better, helps the recipient quite a lot.

Being on the receiving end all the time, I should know ;-).

> As you mentioned in the v2 thread:
>
>   Actually it is up to contributors whether they want to include `--base` 
>   or not.
>
> This is a level of discretion that we shouldn't be leaving to first
> timers (as a very recent first timer, I would not appreciate this
> ambiguity).

Exactly.  The document is about contributing to _this_ project, and
we can and should be more explicit to say what we prefer to see.

Going back and forth is often a good way to figure out what kinks to
iron out and make progress, and in this particular case, it seems it
have already consumed enough bandwidth to make as much progress as we
can make in this approach, so let's start concluding by suggesting a
much simpler rewrite.

How about doing the following and call it done?

----- >8 --------- >8 --------- >8 --------- >8 ----
Subject: [PATCH] MyFirstContribution: teach to use "format-patch --base=auto"

Let's encourage first-time contributors to tell us what commit they
based their work with the format-patch invocation.  As the example
already forks from origin/master and by branch.autosetupmerge by
default records the upstream when the psuh branch was created, we
can use --base=auto for this.  Also, mention the readers that the
range of commits can simply be given with `@{u}` if they are on the
`psuh` branch already.

As we are getting one more option on the command line, and spending
one paragraph each to explain them, let's reformat that part of the
description as a bulletted list.

Signed-off-by: Junio C Hamano <[e-mail]>
---

 * Fell out as an alternative approach during a review by Bagas
   Sanjaya https://lore.kernel.org/git/xmqqo87q6whk.fsf@gitster.g/

 Documentation/MyFirstContribution.txt | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

Comments

Eric Sunshine Oct. 19, 2021, 4:36 a.m. UTC | #1

On Mon, Oct 18, 2021 at 4:09 PM Junio C Hamano <[e-mail]> wrote:
> Subject: [PATCH] MyFirstContribution: teach to use "format-patch --base=auto"
>
> Let's encourage first-time contributors to tell us what commit they
> based their work with the format-patch invocation.  As the example
> already forks from origin/master and by branch.autosetupmerge by

I think you meant: s/and by/and/

> default records the upstream when the psuh branch was created, we
> can use --base=auto for this.  Also, mention the readers that the

s/the readers/to readers/

> range of commits can simply be given with `@{u}` if they are on the
> `psuh` branch already.
>
> As we are getting one more option on the command line, and spending
> one paragraph each to explain them, let's reformat that part of the
> description as a bulletted list.
>
> Signed-off-by: Junio C Hamano <[e-mail]>

Patch

diff --git c/Documentation/MyFirstContribution.txt w/Documentation/MyFirstContribution.txt
index b20bc8e914..e505a7ec87 100644
--- c/Documentation/MyFirstContribution.txt
+++ w/Documentation/MyFirstContribution.txt
@@ -905,19 +905,31 @@  Sending emails with Git is a two-part process; before you can prepare the emails
 themselves, you'll need to prepare the patches. Luckily, this is pretty simple:
 
 ----
-$ git format-patch --cover-letter -o psuh/ master..psuh
+$ git format-patch --cover-letter -o psuh/ --base=auto psuh@{u}..psuh
 ----
 
-The `--cover-letter` parameter tells `format-patch` to create a cover letter
-template for you. You will need to fill in the template before you're ready
-to send - but for now, the template will be next to your other patches.
+ . The `--cover-letter` option tells `format-patch` to create a
+   cover letter template for you. You will need to fill in the
+   template before you're ready to send - but for now, the template
+   will be next to your other patches.
 
-The `-o psuh/` parameter tells `format-patch` to place the patch files into a
-directory. This is useful because `git send-email` can take a directory and
-send out all the patches from there.
+ . The `-o psuh/` option tells `format-patch` to place the patch
+   files into a directory. This is useful because `git send-email`
+   can take a directory and send out all the patches from there.
 
-`master..psuh` tells `format-patch` to generate patches for the difference
-between `master` and `psuh`. It will make one patch file per commit. After you
+ . The `--base=auto` option tells the command to record the "base
+   commit", on which the recipient is expected to apply the patch
+   series.
+
+ . The `psuh@{u}..psuh` option tells `format-patch` to generate
+   patches for the commits you created on the `psuh` branch since it
+   forked from its upstream (which is `origin/master` if you
+   followed the example in the "Set up your workspace" section).  If
+   you are already on the `psuh` branch, you can just say `@{u}`,
+   which means "commits on the current branch since it forked from
+   its upstream", which is the same thing.
+
+The command will make one patch file per commit. After you
 run, you can go have a look at each of the patches with your favorite text
 editor and make sure everything looks alright; however, it's not recommended to
 make code fixups via the patch file. It's a better idea to make the change the