| Message ID | 20211018124106.542050-1-bagasdotme@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] Documentation: specify base point when generating MyFirstContribution patchset | expand |
Bagas Sanjaya <bagasdotme@gmail.com> writes: > Specifying base point (commit hash) can help reviewers and testers > interested on the patchset. Mention how to record it with `--base` > option to `format-patch`. > > Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com> > --- > Changes since v3 [1]: > - rewording (suggested by Glen) > > I don't apply Junio's suggestion that use `--base=auto`, because in > most cases invocations of the option requires full hash of base object > and AFAIK people just do `git checkout -b` without specifying the > tracking option (`-t`). That's actually quite sad, if it were true. Our home, when we added "end-user/newbie friendly features" to "checkout" (credit goes to Dscho, IIRC), was actually most new people would not even have to say "-b" in the simplest tasks (instead they rely on "git checkout foo" gets turned into "git checkout -t -b foo origin/foo" by DWIMmery) and the fact that branch.autoSetupMerge defaulting to 'true' so that they get the @{u} and --base=auto support without even saying "-t" (as long as they do not explicitly decline with "--no-track" from the command line, that is). > -$ git format-patch --cover-letter -o psuh/ master..psuh > +$ git show -s --format="%H" master I said this is not good in my previous response already and told you to teach merge-base, if we were not to use the --base=auto, didn't I? The range given to format-patch, i.e. master..psuh, would be the correct range of commits to format, as long as you didn't rewind the local master branch. 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.
The fact that we are having this right/wrong base discussion makes me think that including the base is out of scope for first time contributors. > 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'? If it matters, the 'master' should be part of the patchset or it can be described using the conventions of the list ("this series is based off a merge of ab/foo-bar and cd/baz"). If it does not matter, then providing the base is not helpful. I suspect that we are documenting --base is that we do not have *any* documentation of this in our mailing list workflow docs, and MyFirstContribution is becoming a catch-all for all of our workflow regardless of whether it is truly for first-timers or not. My own docs changes [1] are arguably guilty of doing the same thing. 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). This document should be recommending good, easy-to-follow defaults for first timers, thus I think that discretionary things belong elsewhere. We might consider adding _yet another_ document designed for levelling up contributors past their first contribution, something along the lines of "Patch submission tips and tricks". This could hold information that we want contributors to know about, but are not necessary for a first-timer e.g. --range-diff and --base. [1] https://lore.kernel.org/git/20210922202218.7986-1-chooglen@google.com/
diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt index b20bc8e914..1c4cd092ee 100644 --- a/Documentation/MyFirstContribution.txt +++ b/Documentation/MyFirstContribution.txt @@ -902,10 +902,19 @@ is out of scope for the context of this tutorial. === Preparing Initial Patchset 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: +themselves, you'll need to prepare the patches. Luckily, this is pretty simple. +First, we need to get hash of the commit the patchset is based on. We call +this commit the `<base>`: ---- -$ git format-patch --cover-letter -o psuh/ master..psuh +$ git show -s --format="%H" master +---- + +Now generate the patchset, passing the hash of `<base>` to the `--base` +parameter: + +---- +$ git format-patch --cover-letter --base=<base> -o psuh/ master..psuh ---- The `--cover-letter` parameter tells `format-patch` to create a cover letter @@ -916,6 +925,10 @@ 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 `--base=<base>` parameter tells `format-patch` to embed base commit +hash to the cover letter. Reviewers and testers interested in the patchset +can create branch based on the specifed base commit in order to apply it. + `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 run, you can go have a look at each of the patches with your favorite text @@ -1046,7 +1059,7 @@ reviewer comments. Once the patch series is ready for submission, generate your patches again, but with some new flags: ---- -$ git format-patch -v2 --cover-letter -o psuh/ --range-diff master..psuh-v1 master.. +$ git format-patch -v2 --cover-letter -o psuh/ --base=<base> --range-diff master..psuh-v1 master.. ---- The `--range-diff master..psuh-v1` parameter tells `format-patch` to include a
Specifying base point (commit hash) can help reviewers and testers interested on the patchset. Mention how to record it with `--base` option to `format-patch`. Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com> --- Changes since v3 [1]: - rewording (suggested by Glen) I don't apply Junio's suggestion that use `--base=auto`, because in most cases invocations of the option requires full hash of base object and AFAIK people just do `git checkout -b` without specifying the tracking option (`-t`). [1]: https://lore.kernel.org/git/xmqqo87q6whk.fsf@gitster.g/T/#t Documentation/MyFirstContribution.txt | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) base-commit: f443b226ca681d87a3a31e245a70e6bc2769123c