Improve MyFirstContribution's GitGitGadget section
PATCH 3/3 MyFirstContribution: drop PR description for GGG single-patch contributions
In reply to Philippe Blain via GitGitGadgetFrom: Philippe Blain <[email protected]>
By default, GitHub prefills the PR description using the commit message for single-commit PRs. This results in a duplicate commit message below the three-dash line if the contributor does not empty out the PR description before submitting, which adds noise for reviewers.
Add a note to that effect in MyFirstContribution.txt.
This partly addresses:
https://github.com/gitgitgadget/gitgitgadget/issues/340
--- Documentation/MyFirstContribution.txt | 8 ++++++++ 1 file changed, 8 insertions(+)
Documentation/MyFirstContribution.txt +8 -0
@@ -821,6 +821,14 @@ Adding the 'psuh' command Your PR's description will used as the body of the cover letter. include::MyFirstContribution-coverletter.txt[] +NOTE: For single-patch contributions, your commit message should already be +meaningful and explain at a high level the purpose (what is happening and why) +of your patch, so you usually do not need any additional context. In that case, +remove the PR description that GitHub automatically generates from your commit +message (your PR description should be empty). If you do need to supply even +more context, you can do so in that space and it will be appended to the email +that GitGitGadget will send, separately from your commit message. + When you're happy, submit your pull request. [[run-ci-ggg]]
Signature · 1 line
gitgitgadget
PATCH 2/3 MyFirstContribution: also explain cover letter in GitGitGadget section
In reply to Philippe Blain via GitGitGadgetFrom: Philippe Blain <[email protected]>
The "Sending Patches via GitGitGadget" section mentions that the PR title and description will be used as the cover letter, but does not explain what is a cover letter or what should be included in it.
Mention the purpose of the cover letter in that section, and give examples for the title and description, leveraging the excerpt extracted from the "Sending Patches with git send-email" section in the previous commit.
--- Documentation/MyFirstContribution.txt | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
Documentation/MyFirstContribution.txt +14 -2
@@ -808,8 +808,20 @@ https://github.com/gitgitgadget/git and open a PR either with the "New pull request" button or the convenient "Compare & pull request" button that may appear with the name of your newly pushed branch. -Review the PR's title and description, as it's used by GitGitGadget as the cover -letter for your change. When you're happy, submit your pull request. +Review the PR's title and description, as they're used by GitGitGadget as the +cover letter for your change. The cover letter describes your proposed +contribution as a whole and is ideal to mention any context that might be +useful for reviewers. The title of your PR should be something which +succinctly covers the purpose of your entire topic branch, for example: + +---- +Adding the 'psuh' command +---- + +Your PR's description will used as the body of the cover letter. +include::MyFirstContribution-coverletter.txt[] + +When you're happy, submit your pull request. [[run-ci-ggg]] === Running CI and Getting Ready to Send
Signature · 1 line
gitgitgadget
PATCH 1/3 MyFirstContribution: move cover letter description to a separate file
In reply to Philippe Blain via GitGitGadgetFrom: Philippe Blain <[email protected]>
In a subsequent commit we want to reuse the explanation of the purpose of The cover letter form the "Sending Patches with git send-email" section in the "Sending Patches via GitGitGadget" section.
To avoid text duplication, move this explanation to a separate file and include it in MyFirstContribution.txt.
--- .../MyFirstContribution-coverletter.txt | 18 ++++++++++++++++ Documentation/MyFirstContribution.txt | 21 ++----------------- 2 files changed, 20 insertions(+), 19 deletions(-) create mode 100644 Documentation/MyFirstContribution-coverletter.txt
Documentation/MyFirstContribution-coverletter.txt new file +18 -0
@@ -0,0 +1,18 @@ +This is an important component of change submission as it explains to the +community from a high level what you're trying to do, and why, in a way that's +more apparent than just looking at your diff. Be sure to explain anything your +diff doesn't make clear on its own. + +Here's an example body for `psuh`: + +---- +Our internal metrics indicate widespread interest in the command +git-psuh - that is, many users are trying to use it, but finding it is +unavailable, using some unknown workaround instead. + +The following handful of patches add the psuh command and implement some +handy features on top of it. + +This patchset is part of the MyFirstContribution tutorial and should not +be merged. +----
Documentation/MyFirstContribution.txt +2 -19
@@ -976,25 +976,8 @@ filter their email for this type of flag. You'll need to add some extra parameters when you invoke `git send-email` to add the cover letter. -Next you'll have to fill out the body of your cover letter. This is an important -component of change submission as it explains to the community from a high level -what you're trying to do, and why, in a way that's more apparent than just -looking at your diff. Be sure to explain anything your diff doesn't make clear -on its own. - -Here's an example body for `psuh`: - ----- -Our internal metrics indicate widespread interest in the command -git-psuh - that is, many users are trying to use it, but finding it is -unavailable, using some unknown workaround instead. - -The following handful of patches add the psuh command and implement some -handy features on top of it. - -This patchset is part of the MyFirstContribution tutorial and should not -be merged. ----- +Next you'll have to fill out the body of your cover letter. +include::MyFirstContribution-coverletter.txt[] The template created by `git format-patch --cover-letter` includes a diffstat. This gives reviewers a summary of what they're in for when reviewing your topic.
Signature · 1 line
gitgitgadget
cover Improve MyFirstContribution's GitGitGadget section
Two small improvements to the MyFirstContribution tutorial:
* Describe the purpose of the cover letter in that section also, and give an example just as in the 'git send-email' section
* Instruct contributors to drop the GitHub-generated PR description for single patch contributions.
Philippe Blain (3):
MyFirstContribution: move cover letter description to a separate file
MyFirstContribution: also explain cover letter in GitGitGadget section
MyFirstContribution: drop PR description for GGG single-patch
contributions
.../MyFirstContribution-coverletter.txt | 18 ++++++++ Documentation/MyFirstContribution.txt | 45 ++++++++++--------- 2 files changed, 42 insertions(+), 21 deletions(-) create mode 100644 Documentation/MyFirstContribution-coverletter.txt
base-commit: 6cd33dceed60949e2dbc32e3f0f5e67c4c882e1e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1226%2Fphil-blain%2Fmyfirst-contrib-single-patch-ggg-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1226/phil-blain/myfirst-contrib-single-patch-ggg-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1226
Signature · 1 line
gitgitgadget
Re: [PATCH 2/3] MyFirstContribution: also explain cover letter in GitGitGadget section
In reply to Philippe Blain via GitGitGadgetQuoted 24 lines · Philippe Blain via GitGitGadget
From: Philippe Blain <[email protected]>
The "Sending Patches via GitGitGadget" section mentions that the PR title and description will be used as the cover letter, but does not explain what is a cover letter or what should be included in it.
Mention the purpose of the cover letter in that section, and give examples for the title and description, leveraging the excerpt extracted from the "Sending Patches with git send-email" section in the previous commit.
Signed-off-by Philippe Blain <[email protected]>--- Documentation/MyFirstContribution.txt | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)Documentation/MyFirstContribution.txt +0
-0@@ -808,8 +808,20 @@ https://github.com/gitgitgadget/git and open a PR either with the "New pull request" button or the convenient "Compare & pull request" button that may appear with the name of your newly pushed branch.
Documentation/MyFirstContribution.txt https://github.com/gitgitgadget/git and open a PR either with the "New pull · line 818
Quoted 10 lines
Documentation/MyFirstContribution.txt +8
-2-Review the PR's title and description, as it's used by GitGitGadget as the cover -letter for your change. When you're happy, submit your pull request. +Review the PR's title and description, as they're used by GitGitGadget as the +cover letter for your change. The cover letter describes your proposed +contribution as a whole and is ideal to mention any context that might be +useful for reviewers. The title of your PR should be something which +succinctly covers the purpose of your entire topic branch, for example: + +---- +Adding the 'psuh' command
Typically I see patch series titles follow the same "imperative mood" as commit titles/messages (see 'Documentation/SubmittingPatches.txt'). I'm not sure whether that's a rule written down somewhere or just convention, but for the sake of consistency you might want to do something like:
"Add the 'psuh' command"
Documentation/MyFirstContribution.txt https://github.com/gitgitgadget/git and open a PR either with the "New pull · line 821
Quoted 3 lines
Documentation/MyFirstContribution.txt +3
-0+---- + +Your PR's description will used as the body of the cover letter.
Including the line "Your PR's description..." is somewhat confusing to me as a first-time reader, since I was interpreting this section to be the *verbatim* text of the pull request title & description. If this *is* meant to be that description, then the note about the PR description can be removed. That point is also mentioned above, so it's probably not needed here anyway.
Quoted 6 lines
Documentation/MyFirstContribution.txt +3
-0+include::MyFirstContribution-coverletter.txt[] + +When you're happy, submit your pull request. [[run-ci-ggg]] === Running CI and Getting Ready to Send
Re: [PATCH 1/3] MyFirstContribution: move cover letter description to a separate file
In reply to Philippe Blain via GitGitGadgetQuoted 2 lines · Philippe Blain via GitGitGadget
In a subsequent commit we want to reuse the explanation of the purpose of The cover letter form the "Sending Patches with git send-email" section
s/The/the
s/form/from/
Quoted 6 lines
in the "Sending Patches via GitGitGadget" section.
To avoid text duplication, move this explanation to a separate file and include it in MyFirstContribution.txt.
Signed-off-by Philippe Blain <[email protected]>
Re: [PATCH 1/3] MyFirstContribution: move cover letter description to a separate file
In reply to Philippe Blain via GitGitGadgetQuoted 9 lines · Philippe Blain via GitGitGadget
From: Philippe Blain <[email protected]>
In a subsequent commit we want to reuse the explanation of the purpose of The cover letter form the "Sending Patches with git send-email" section in the "Sending Patches via GitGitGadget" section.
To avoid text duplication, move this explanation to a separate file and include it in MyFirstContribution.txt.
Quoted 6 lines
Signed-off-by Philippe Blain <[email protected]>--- .../MyFirstContribution-coverletter.txt | 18 ++++++++++++++++ Documentation/MyFirstContribution.txt | 21 ++----------------- 2 files changed, 20 insertions(+), 19 deletions(-) create mode 100644 Documentation/MyFirstContribution-coverletter.txt
If the end-product is a single document, "MyFirstContribution", I am skeptical if it is a good change to duplicate the same text in two sections.
Should we instead reorganize the document so these concepts like cover letters, log message for each commit, etc., which are applicable regardless of how you send them out, are explained, before we start talking about "now, we understand the components out of which a patch series is made of, here is how you give it a final polish and send it out, with send-email and/or with GGG"?
Short of that, perhaps whichever appears later in the document that uses "cover letter" can just say "see above"?
Re: [PATCH 3/3] MyFirstContribution: drop PR description for GGG single-patch contributions
In reply to Philippe Blain via GitGitGadgetDocumentation/MyFirstContribution.txt Adding the 'psuh' command · line 830
Quoted 7 lines · Philippe Blain via GitGitGadget
Documentation/MyFirstContribution.txt +7
-0+NOTE: For single-patch contributions, your commit message should already be +meaningful and explain at a high level the purpose (what is happening and why) +of your patch, so you usually do not need any additional context. In that case, +remove the PR description that GitHub automatically generates from your commit +message (your PR description should be empty). If you do need to supply even +more context, you can do so in that space and it will be appended to the email +that GitGitGadget will send, separately from your commit message.
"separately from your commit message" wants to be clarified. It sounds as if GGG will send a separate message.
I am _guessing_ that you meant something like "the body of your proposed log message ends with your sign-off and followed by a line with three-dashes on it. After that three-dash line, and before the diffstat and the patch, is an appropriate place to write additional information that are meant to help reviewers during review but will become irrelevant after the review is done. Your PR description will appear there for a single patch contribution".
If we do not explain the space after the three-dash line elsewhere, perhaps we should do so separately, like we talk about cover letters.
Thanks.
Re: [PATCH 1/3] MyFirstContribution: move cover letter description to a separate file
In reply to Eric SunshineHi Eric,
Le 2022-04-27 à 17:15, Eric Sunshine a écrit :
Quoted 8 lines
Quoted 2 lines · Philippe Blain via GitGitGadget
In a subsequent commit we want to reuse the explanation of the purpose of The cover letter form the "Sending Patches with git send-email" section
s/The/the
s/form/from/
Indeed, will fix. Thanks,
Philippe.
Re: [PATCH 1/3] MyFirstContribution: move cover letter description to a separate file
In reply to Junio C HamanoHi Junio,
Le 2022-04-27 à 17:48, Junio C Hamano a écrit :
Quoted 29 lines
Quoted 9 lines · Philippe Blain via GitGitGadget
From: Philippe Blain <[email protected]>
In a subsequent commit we want to reuse the explanation of the purpose of The cover letter form the "Sending Patches with git send-email" section in the "Sending Patches via GitGitGadget" section.
To avoid text duplication, move this explanation to a separate file and include it in MyFirstContribution.txt.
Quoted 6 lines
Signed-off-by Philippe Blain <[email protected]>--- .../MyFirstContribution-coverletter.txt | 18 ++++++++++++++++ Documentation/MyFirstContribution.txt | 21 ++----------------- 2 files changed, 20 insertions(+), 19 deletions(-) create mode 100644 Documentation/MyFirstContribution-coverletter.txtIf the end-product is a single document, "MyFirstContribution", I am skeptical if it is a good change to duplicate the same text in two sections.
Should we instead reorganize the document so these concepts like cover letters, log message for each commit, etc., which are applicable regardless of how you send them out, are explained, before we start talking about "now, we understand the components out of which a patch series is made of, here is how you give it a final polish and send it out, with send-email and/or with GGG"?
Quoted 3 lines
Short of that, perhaps whichever appears later in the document that uses "cover letter" can just say "see above"?
Yeah, I guess you are right. I choosed to factor out the cover letter part like this mostly because it was a smaller change. I'll think of doing it like you suggest.
Thanks,
Philippe.
Re: [PATCH 2/3] MyFirstContribution: also explain cover letter in GitGitGadget section
In reply to Victoria DyeHi Victoria,
Le 2022-04-27 à 16:43, Victoria Dye a écrit :
Quoted 5 lines
Quoted 2 lines · Philippe Blain via GitGitGadget
+----
+Adding the 'psuh' command
Quoted 7 lines
Typically I see patch series titles follow the same "imperative mood" as commit titles/messages (see '
Documentation/SubmittingPatches.txt'). I'm not sure whether that's a rule written down somewhere or just convention, but for the sake of consistency you might want to do something like:"Add the 'psuh' command"
I fully agree. I just copied the existing patch series title from the git-send-email section further down. I think it would make sense to change this to also using the imperative mood just like commit messages in a preparatory commit. I'll do that.
Quoted 10 lines
Review
Quoted 3 lines
+----
(quoted diff) +2
-0+ +Your PR's description will used as the body of the cover letter.Including the line "Your PR's description..." is somewhat confusing to me as a first-time reader, since I was interpreting this section to be the *verbatim* text of the pull request title & description. If this *is* meant to be that description, then the note about the PR description can be removed. That point is also mentioned above, so it's probably not needed here anyway.
I'm not exactly sure what you mean. I meant that the description of the PR will be used as the body of the cover letter...
Thanks for having a look!
Philippe.
Re: [PATCH 3/3] MyFirstContribution: drop PR description for GGG single-patch contributions
In reply to Junio C HamanoLe 2022-04-27 à 17:56, Junio C Hamano a écrit :
Quoted 13 lines
Documentation/MyFirstContribution.txt Adding the 'psuh' command · line 830
Quoted 7 lines · Philippe Blain via GitGitGadget
Documentation/MyFirstContribution.txt +7
-0+NOTE: For single-patch contributions, your commit message should already be +meaningful and explain at a high level the purpose (what is happening and why) +of your patch, so you usually do not need any additional context. In that case, +remove the PR description that GitHub automatically generates from your commit +message (your PR description should be empty). If you do need to supply even +more context, you can do so in that space and it will be appended to the email +that GitGitGadget will send, separately from your commit message."separately from your commit message" wants to be clarified. It sounds as if GGG will send a separate message.
Quoted 7 lines
I am _guessing_ that you meant something like "the body of your proposed log message ends with your sign-off and followed by a line with three-dashes on it. After that three-dash line, and before the diffstat and the patch, is an appropriate place to write additional information that are meant to help reviewers during review but will become irrelevant after the review is done. Your PR description will appear there for a single patch contribution".
Yes, this is what I meant. I'll try to phrase it differently.
Quoted 4 lines
If we do not explain the space after the three-dash line elsewhere, perhaps we should do so separately, like we talk about cover letters.
Yeah, it is explained in the "Bonus Chapter: One-Patch Changes" subsection [1] at the end of the "Sending Patches with git send-email" section, so I guess people only reading the GitGitGadget instructions will not read that far...
Considering your other comment, I think that a little section titled something like "Anatomy of a patch series" would make sense to introduce how then end product looks like and then the sections on GGG or git-send-email can reference this earlier section.
Good idea.
Thanks!
Philippe.
[1] https://git-scm.com/docs/MyFirstContribution#single-patch
Re: [PATCH 3/3] MyFirstContribution: drop PR description for GGG single-patch contributions
Quoted 4 lines · Philippe Blain
Considering your other comment, I think that a little section titled something like "Anatomy of a patch series" would make sense to introduce how then end product looks like and then the sections on GGG or git-send-email can reference this earlier section.
"Anatomy" does sound like a good idea.
Re: [PATCH 2/3] MyFirstContribution: also explain cover letter in GitGitGadget section
In reply to Philippe BlainQuoted 32 lines · Philippe Blain
Hi Victoria,
Le 2022-04-27 à 16:43, Victoria Dye a écrit :
Quoted 5 lines
Quoted 2 lines · Philippe Blain via GitGitGadget
+----
+Adding the 'psuh' commandQuoted 7 lines
Typically I see patch series titles follow the same "imperative mood" as commit titles/messages (see '
Documentation/SubmittingPatches.txt'). I'm not sure whether that's a rule written down somewhere or just convention, but for the sake of consistency you might want to do something like:"Add the 'psuh' command"
I fully agree. I just copied the existing patch series title from the git-send-email section further down. I think it would make sense to change this to also using the imperative mood just like commit messages in a preparatory commit. I'll do that.
Quoted 10 lines
Review
Quoted 3 lines
+----
(quoted diff) +2
-0+ +Your PR's description will used as the body of the cover letter.Including the line "Your PR's description..." is somewhat confusing to me as a first-time reader, since I was interpreting this section to be the *verbatim* text of the pull request title & description. If this *is* meant to be that description, then the note about the PR description can be removed. That point is also mentioned above, so it's probably not needed here anyway.
Quoted 3 lines
I'm not exactly sure what you mean. I meant that the description of the PR will be used as the body of the cover letter...
Sorry for being unclear. What I meant was that, the way this part of the document reads (to me) right now, it looks like the recommendation is to create a pull request with the title & description:
Title:
Adding the 'psuh' command
Description:
Your PR's description will [be] used as the body of the cover letter. <insert content from MyFirstContribution-coverletter.txt>
That is, the line "Your PR description will..." is the first line of the description (which I don't think was your intention).
That said, upon a more holistic re-read, I've found a few more things that are confusing/oddly phrased. I'll try re-reviewing (part of) the patch below, with (hopefully) more direct/concrete feedback this time:
Documentation/MyFirstContribution.txt https://github.com/gitgitgadget/git and open a PR either with the "New pull · line 813
Quoted 5 lines
Documentation/MyFirstContribution.txt +3
-2-Review the PR's title and description, as it's used by GitGitGadget as the cover -letter for your change. When you're happy, submit your pull request. +Review the PR's title and description, as they're used by GitGitGadget as the +cover letter for your change. The cover letter describes your proposed +contribution as a whole and is ideal to mention any context that might be
s/is ideal to/should(?)
Documentation/MyFirstContribution.txt https://github.com/gitgitgadget/git and open a PR either with the "New pull · line 819
Quoted 6 lines
Documentation/MyFirstContribution.txt +6
-0+useful for reviewers. The title of your PR should be something which +succinctly covers the purpose of your entire topic branch, for example: + +---- +Adding the 'psuh' command +----
Other than the "Adding" -> "Add" change, this is good - the example title will be formatted as a monospaced block, clearly separating it from the advice/context preceding it.
Documentation/MyFirstContribution.txt https://github.com/gitgitgadget/git and open a PR either with the "New pull · line 823
Quoted 4 lines
Documentation/MyFirstContribution.txt +4
-0+ +Your PR's description will used as the body of the cover letter. +include::MyFirstContribution-coverletter.txt[] +
Conversely, this section has no visible separation between the context ("Your PR's description...") and the example ("include::MyFirstContribution-coverletter.txt[]"), hence my confusion earlier.
To parallel the title example, maybe you could use similar monospace formatting for the example? E.g.:
+
+Your PR's description will used as the body of the cover letter. +
+----
+include::MyFirstContribution-coverletter.txt[]
+----
+
One thing I noticed on my re-read is that the context you provide for the title ("The title of your PR should be something...") isn't quite paralleled in the context for the description ("Your PR's description..."). The former talks about *how* you should write your cover letter title, whereas the latter simply states that the PR description will become the cover letter's content. For added context, then, you might want to describe how the cover letter content should be written, e.g.:
+
+Your PR's description will be used as the body of the cover letter and +should explain the purpose of the series, for example:
+
+----
+include::MyFirstContribution-coverletter.txt[]
+----
+
(note the s/will used/will be used)
Apologies for all the picky grammar comments - this series includes a lot of really helpful information for new contributors, and I really appreciate your work here! Hopefully my feedback is a bit more clear this time (and is relatively straightforward to implement).
Thanks!
Quoted 3 lines
Thanks for having a look!
Philippe.
Re: [PATCH 2/3] MyFirstContribution: also explain cover letter in GitGitGadget section
Hi Victoria,
Le 2022-04-29 à 12:27, Victoria Dye a écrit :
Quoted 111 lines
Quoted 32 lines · Philippe Blain
Hi Victoria,
Le 2022-04-27 à 16:43, Victoria Dye a écrit :
Quoted 5 lines
Quoted 2 lines · Philippe Blain via GitGitGadget
+----
+Adding the 'psuh' commandQuoted 7 lines
Typically I see patch series titles follow the same "imperative mood" as commit titles/messages (see '
Documentation/SubmittingPatches.txt'). I'm not sure whether that's a rule written down somewhere or just convention, but for the sake of consistency you might want to do something like:"Add the 'psuh' command"
I fully agree. I just copied the existing patch series title from the git-send-email section further down. I think it would make sense to change this to also using the imperative mood just like commit messages in a preparatory commit. I'll do that.
Quoted 10 lines
Review
Quoted 3 lines
+----
(quoted diff) +2
-0+ +Your PR's description will used as the body of the cover letter.Including the line "Your PR's description..." is somewhat confusing to me as a first-time reader, since I was interpreting this section to be the *verbatim* text of the pull request title & description. If this *is* meant to be that description, then the note about the PR description can be removed. That point is also mentioned above, so it's probably not needed here anyway.
Quoted 3 lines
I'm not exactly sure what you mean. I meant that the description of the PR will be used as the body of the cover letter...
Sorry for being unclear. What I meant was that, the way this part of the document reads (to me) right now, it looks like the recommendation is to create a pull request with the title & description:
Title:
Adding the 'psuh' command
Description:
Your PR's description will [be] used as the body of the cover letter. <insert content from MyFirstContribution-coverletter.txt>That is, the line "Your PR description will..." is the first line of the description (which I don't think was your intention).
That said, upon a more holistic re-read, I've found a few more things that are confusing/oddly phrased. I'll try re-reviewing (part of) the patch below, with (hopefully) more direct/concrete feedback this time:
Documentation/MyFirstContribution.txt https://github.com/gitgitgadget/git and open a PR either with the "New pull · line 813
Quoted 5 lines
Documentation/MyFirstContribution.txt +3
-2-Review the PR's title and description, as it's used by GitGitGadget as the cover -letter for your change. When you're happy, submit your pull request. +Review the PR's title and description, as they're used by GitGitGadget as the +cover letter for your change. The cover letter describes your proposed +contribution as a whole and is ideal to mention any context that might bes/is ideal to/should(?)
Documentation/MyFirstContribution.txt https://github.com/gitgitgadget/git and open a PR either with the "New pull · line 819
Quoted 6 lines
Documentation/MyFirstContribution.txt +6
-0+useful for reviewers. The title of your PR should be something which +succinctly covers the purpose of your entire topic branch, for example: + +---- +Adding the 'psuh' command +----Other than the "Adding" -> "Add" change, this is good - the example title will be formatted as a monospaced block, clearly separating it from the advice/context preceding it.
Documentation/MyFirstContribution.txt https://github.com/gitgitgadget/git and open a PR either with the "New pull · line 823
Quoted 4 lines
Documentation/MyFirstContribution.txt +4
-0+ +Your PR's description will used as the body of the cover letter. +include::MyFirstContribution-coverletter.txt[] +Conversely, this section has no visible separation between the context ("Your PR's description...") and the example ("include::MyFirstContribution-coverletter.txt[]"), hence my confusion earlier.
To parallel the title example, maybe you could use similar monospace formatting for the example? E.g.:
Documentation/MyFirstContribution.txt +7
-0+ +Your PR's description will used as the body of the cover letter. + +---- +include::MyFirstContribution-coverletter.txt[] +---- +One thing I noticed on my re-read is that the context you provide for the title ("The title of your PR should be something...") isn't quite paralleled in the context for the description ("Your PR's description..."). The former talks about *how* you should write your cover letter title, whereas the latter simply states that the PR description will become the cover letter's content. For added context, then, you might want to describe how the cover letter content should be written, e.g.:
Documentation/MyFirstContribution.txt +8
-0+ +Your PR's description will be used as the body of the cover letter and +should explain the purpose of the series, for example: + +---- +include::MyFirstContribution-coverletter.txt[] +---- +
Quoted 6 lines
(note the s/will used/will be used)
Apologies for all the picky grammar comments - this series includes a lot of really helpful information for new contributors, and I really appreciate your work here! Hopefully my feedback is a bit more clear this time (and is relatively straightforward to implement).
Thanks for your feedback. I'll send an updated and expanded version soon and I think I've adressed the different things you noted.
Cheers,
Philippe.
v2 cover Improve MyFirstContribution's GitGitGadget section
In reply to Philippe Blain via GitGitGadgetThanks a lot for the reviews! Here is an updated version.
Changes since v1:
* Based on v1 feedback, instead of reusing the 'git send-email' content verbatim in the GGG section, added a new section and reference it in both 'git send-email' and GGG sections. (patches 1/5-4/5)
* In patch 5/5 (patch 3/3 in v1), tweak wording and add a reference to the "Bonus Chapter: One-Patch Changes" section.
v1: Two small improvements to the MyFirstContribution tutorial:
* Describe the purpose of the cover letter in that section also, and give an example just as in the 'git send-email' section
* Instruct contributors to drop the GitHub-generated PR description for single patch contributions.
Philippe Blain (5):
MyFirstContribution: add "Anatomy of a Patch Series" section
MyFirstContribution: add standalone section on cover letter
MyFirstContribution: reference "The cover letter" in "Preparing Email"
MyFirstContribution: reference "The cover letter" in GitGitGadget
section
MyFirstContribution: drop PR description for GGG single-patch
contributions
Documentation/MyFirstContribution.txt | 149 ++++++++++++++++++++------ 1 file changed, 114 insertions(+), 35 deletions(-)
base-commit: 6cd33dceed60949e2dbc32e3f0f5e67c4c882e1e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1226%2Fphil-blain%2Fmyfirst-contrib-single-patch-ggg-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1226/phil-blain/myfirst-contrib-single-patch-ggg-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1226
Range-diff vs v1:
-: ----------- > 1: 59af7e5e5ad MyFirstContribution: add "Anatomy of a Patch Series" section -: ----------- > 2: 9552d80a80d MyFirstContribution: add standalone section on cover letter 1: 7e84d5b286d ! 3: d7699289ea6 MyFirstContribution: move cover letter description to a separate file
@@ Metadata
Author: Philippe Blain <[email protected]>
## Commit message ##
- MyFirstContribution: move cover letter description to a separate file
+ MyFirstContribution: reference "The cover letter" in "Preparing Email"
- In a subsequent commit we want to reuse the explanation of the purpose of
- The cover letter form the "Sending Patches with git send-email" section
- in the "Sending Patches via GitGitGadget" section.
+ The previous commit added a standalone section on the purpose of the
+ cover letter, drawing inspiration from the existing content of the
+ "Preparing Email" section.
- To avoid text duplication, move this explanation to a separate file and
- include it in MyFirstContribution.txt.
+ Adjust "Preparing Email" to reference "The cover letter", to avoid
+ content duplication.
- Signed-off-by: Philippe Blain <[email protected]>
+ Also, use the imperative mode for the cover letter subject, as is done
+ in "The cover letter".
- ## Documentation/MyFirstContribution-coverletter.txt (new) ## -@@
-+This is an important component of change submission as it explains to the -+community from a high level what you're trying to do, and why, in a way that's -+more apparent than just looking at your diff. Be sure to explain anything your -+diff doesn't make clear on its own.
-+
-+Here's an example body for psuh:
-+
-+----
-+Our internal metrics indicate widespread interest in the command -+git-psuh - that is, many users are trying to use it, but finding it is -+unavailable, using some unknown workaround instead.
-+
-+The following handful of patches add the psuh command and implement some -+handy features on top of it.
-+
-+This patchset is part of the MyFirstContribution tutorial and should not -+be merged.
-+----
+ Signed-off-by: Philippe Blain <[email protected]>
## Documentation/MyFirstContribution.txt ##
-@@ Documentation/MyFirstContribution.txt: filter their email for this type of flag.
+@@ Documentation/MyFirstContribution.txt: directory you specified - you're nearly ready to send out your review!
+ [[preparing-cover-letter]]
+ === Preparing Email
+
+-In addition to an email per patch, the Git community also expects your patches
+-to come with a cover letter, typically with a subject line [PATCH 0/x] (where
+-x is the number of patches you're sending). Since you invoked format-patch
+-with --cover-letter, you've already got a template ready. Open it up in your
+-favorite editor.
++Since you invoked format-patch with --cover-letter, you've already got a
++cover letter template ready. Open it up in your favorite editor.
+
+ You should see a number of headers present already. Check that your From:
+-header is correct. Then modify your Subject: to something which succinctly
+-covers the purpose of your entire topic branch, for example:
++header is correct. Then modify your Subject: (see <<cover-letter,above>> for
++how to choose good title for your patch series):
+
+ ----
+-Subject: [PATCH 0/7] adding the 'psuh' command
++Subject: [PATCH 0/7] Add the 'psuh' command
+ ----
+
+ Make sure you retain the ``[PATCH 0/X]'' part; that's what indicates to the Git
+-community that this email is the beginning of a review, and many reviewers
+-filter their email for this type of flag.
++community that this email is the beginning of a patch series, and many
++reviewers filter their email for this type of flag.
+
You'll need to add some extra parameters when you invoke git send-email to add
the cover letter.
@@ Documentation/MyFirstContribution.txt: filter their email for this type of flag.
-This patchset is part of the MyFirstContribution tutorial and should not -be merged.
-----
-+Next you'll have to fill out the body of your cover letter. -+include::MyFirstContribution-coverletter.txt[]
++Next you'll have to fill out the body of your cover letter. Again, see ++<<cover-letter,above>> for what content to include.
The template created by git format-patch --cover-letter includes a diffstat. This gives reviewers a summary of what they're in for when reviewing your topic.
2: afb80b8e9ee ! 4: f6034b0964b MyFirstContribution: also explain cover letter in GitGitGadget section
@@ Metadata
Author: Philippe Blain <[email protected]>
## Commit message ##
- MyFirstContribution: also explain cover letter in GitGitGadget section
+ MyFirstContribution: reference "The cover letter" in GitGitGadget section
The "Sending Patches via GitGitGadget" section mentions that the PR title and description will be used as the cover letter, but does not explain what is a cover letter or what should be included in it.
- Mention the purpose of the cover letter in that section, and give
- examples for the title and description, leveraging the excerpt extracted
- from the "Sending Patches with git send-email" section in the previous
+ Refer readers to the new "The cover letter" section added in a previous
commit.
@@ Documentation/MyFirstContribution.txt: https://github.com/gitgitgadget/git and o
-Review the PR's title and description, as it's used by GitGitGadget as the cover -letter for your change. When you're happy, submit your pull request.
-+Review the PR's title and description, as they're used by GitGitGadget as the -+cover letter for your change. The cover letter describes your proposed -+contribution as a whole and is ideal to mention any context that might be -+useful for reviewers. The title of your PR should be something which -+succinctly covers the purpose of your entire topic branch, for example:
-+
-+----
-+Adding the 'psuh' command
-+----
-+
-+Your PR's description will used as the body of the cover letter. -+include::MyFirstContribution-coverletter.txt[]
++Review the PR's title and description, as they're used by GitGitGadget ++respectively as the subject and body of the cover letter for your change. Refer ++to <<cover-letter,"The cover letter">> above for advice on how to title your ++submission and what content to include in the description.
+
+When you're happy, submit your pull request.
3: 2f6ecbf2601 ! 5: 33256c6b4ba MyFirstContribution: drop PR description for GGG single-patch contributions
@@ Commit message
## Documentation/MyFirstContribution.txt ##
-@@ Documentation/MyFirstContribution.txt: Adding the 'psuh' command
- Your PR's description will used as the body of the cover letter.
- include::MyFirstContribution-coverletter.txt[]
+@@ Documentation/MyFirstContribution.txt: respectively as the subject and body of the cover letter for your change. Refer
+ to <<cover-letter,"The cover letter">> above for advice on how to title your
+ submission and what content to include in the description.
+NOTE: For single-patch contributions, your commit message should already be +meaningful and explain at a high level the purpose (what is happening and why)
@@ Documentation/MyFirstContribution.txt: Adding the 'psuh' command
+remove the PR description that GitHub automatically generates from your commit +message (your PR description should be empty). If you do need to supply even +more context, you can do so in that space and it will be appended to the email
-+that GitGitGadget will send, separately from your commit message. ++that GitGitGadget will send, between the three-dash line and the diffstat ++(see <<single-patch,Bonus Chapter: One-Patch Changes>> for how this looks once ++submitted).
+
When you're happy, submit your pull request.
Signature · 1 line
gitgitgadget
PATCH v2 1/5 MyFirstContribution: add "Anatomy of a Patch Series" section
From: Philippe Blain <[email protected]>
Before describing how to send patches to the mailing list either with GitGitGadget or 'git send-email', the MyFirstContribution tutorial includes a small "Getting Ready to Share" section where the two different methods are briefly introduced.
Use this section to also describe what a patch series looks like once submitted, so that readers get an understanding of the end result before diving into how to accomplish that end result.
Start by copying the "thread overview" section of a recent contribution from the public-inbox web UI and explaining how each commit is a separate mail, and point out the cover letter.
Subsequent commits will move the existing description of the purpose of the cover letter from the 'git send-email' section to this "anatomy" section.
Also, change the wording in the introductory paragraph to use "contributions" instead of "patches", since this makes more sense when talking about GitHub pull requests.
--- Documentation/MyFirstContribution.txt | 54 +++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 3 deletions(-)
Documentation/MyFirstContribution.txt +51 -3
@@ -710,13 +710,61 @@ dependencies. `prove` also makes the output nicer. Go ahead and commit this change, as well. [[ready-to-share]] -== Getting Ready to Share +== Getting Ready to Share: Anatomy of a Patch Series You may have noticed already that the Git project performs its code reviews via emailed patches, which are then applied by the maintainer when they are ready -and approved by the community. The Git project does not accept patches from +and approved by the community. The Git project does not accept contributions from pull requests, and the patches emailed for review need to be formatted a -specific way. At this point the tutorial diverges, in order to demonstrate two +specific way. + +:patch-series: https://lore.kernel.org/git/[email protected]/ +:lore: https://lore.kernel.org/git/ + +Before taking a look at how to convert your commits into emailed patches, +let's analyze what the end result, a "patch series", looks like. Here is an +{patch-series}[example] of the summary view for a patch series on the web interface of +the {lore}[Git mailing list archive]: + +---- +2022-02-18 18:40 [PATCH 0/3] libify reflog John Cai via GitGitGadget +2022-02-18 18:40 ` [PATCH 1/3] reflog: libify delete reflog function and helpers John Cai via GitGitGadget +2022-02-18 19:10 ` Ævar Arnfjörð Bjarmason [this message] +2022-02-18 19:39 ` Taylor Blau +2022-02-18 19:48 ` Ævar Arnfjörð Bjarmason +2022-02-18 19:35 ` Taylor Blau +2022-02-21 1:43 ` John Cai +2022-02-21 1:50 ` Taylor Blau +2022-02-23 19:50 ` John Cai +2022-02-18 20:00 ` // other replies ellided +2022-02-18 18:40 ` [PATCH 2/3] reflog: call reflog_delete from reflog.c John Cai via GitGitGadget +2022-02-18 19:15 ` Ævar Arnfjörð Bjarmason +2022-02-18 20:26 ` Junio C Hamano +2022-02-18 18:40 ` [PATCH 3/3] stash: call reflog_delete from reflog.c John Cai via GitGitGadget +2022-02-18 19:20 ` Ævar Arnfjörð Bjarmason +2022-02-19 0:21 ` Taylor Blau +2022-02-22 2:36 ` John Cai +2022-02-22 10:51 ` Ævar Arnfjörð Bjarmason +2022-02-18 19:29 ` [PATCH 0/3] libify reflog Ævar Arnfjörð Bjarmason +2022-02-22 18:30 ` [PATCH v2 0/3] libify reflog John Cai via GitGitGadget +2022-02-22 18:30 ` [PATCH v2 1/3] stash: add test to ensure reflog --rewrite --updatref behavior John Cai via GitGitGadget +2022-02-23 8:54 ` Ævar Arnfjörð Bjarmason +2022-02-23 21:27 ` Junio C Hamano +// continued +---- + +We can note a few things: + +- Each commit is sent as a separate email, with the commit message title as + subject, prefixed with "[PATCH _i_/_n_]" for the _i_-th commit of an + _n_-commit series. +- Each patch is sent as a reply to an introductory email called the _cover + letter_ of the series, prefixed "[PATCH 0/_n_]". +- Subsequent iterations of the patch series are labelled "[PATCH v2]", "[PATCH + v3]", etc. and sent with a new cover letter, itself a reply to the cover + letter of the previous iteration (more on that below). + +At this point the tutorial diverges, in order to demonstrate two different methods of formatting your patchset and getting it reviewed. The first method to be covered is GitGitGadget, which is useful for those
Signature · 1 line
gitgitgadget
PATCH v2 2/5 MyFirstContribution: add standalone section on cover letter
In reply to Philippe Blain via GitGitGadgetFrom: Philippe Blain <[email protected]>
An explanation of the purpose of the cover letter is included in the "Sending Patches with git send-email" / "Preparing Email" section but is missing from the "Sending Patches via GitGitGadget" section.
Add a standalone section "The cover letter" under the "Getting Started:
Anatomy of a Patch Series" header to explain what the cover letter is used for and to draft the cover letter of the 'psuh' topic used in the tutorial.
For now we mostly copy content from the "Sending Patches with git send-email" section but do not adjust that section, nor the GGG section, to reference the new section. This is done in following commits.
Also, adjust the "Preparing Email" Asciidoc anchor to avoid conflicts.
--- Documentation/MyFirstContribution.txt | 39 ++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-)
Documentation/MyFirstContribution.txt +38 -1
@@ -764,6 +764,43 @@ We can note a few things: v3]", etc. and sent with a new cover letter, itself a reply to the cover letter of the previous iteration (more on that below). +[[cover-letter]] +=== The cover letter + +In addition to an email per patch, the Git community also expects your patches +to come with a cover letter. This is an important component of change +submission as it explains to the community from a high level what you're trying +to do, and why, in a way that's more apparent than just looking at your +patches. + +The title of your cover letter should be something which succinctly covers the +purpose of your entire topic branch. It's often in the imperative mood, just +like our commit message titles. Here is how we'll title our series: + +--- +Add the 'psuh' command +--- + +The body of the cover letter is used to give additional context to reviewers. +Be sure to explain anything your patches don't make clear on their own, but +remember that since the cover letter is not recorded in the commit history, +anything that might be useful to future readers of the repository's history +should be in your commit messages, not in your cover letter. + +Here's an example body for `psuh`: + +---- +Our internal metrics indicate widespread interest in the command +git-psuh - that is, many users are trying to use it, but finding it is +unavailable, using some unknown workaround instead. + +The following handful of patches add the psuh command and implement some +handy features on top of it. + +This patchset is part of the MyFirstContribution tutorial and should not +be merged. +---- + At this point the tutorial diverges, in order to demonstrate two different methods of formatting your patchset and getting it reviewed.
@@ -1000,7 +1037,7 @@ but want reviewers to look at what they have so far. You can add this flag with Check and make sure that your patches and cover letter template exist in the directory you specified - you're nearly ready to send out your review! -[[cover-letter]] +[[preparing-cover-letter]] === Preparing Email In addition to an email per patch, the Git community also expects your patches
Signature · 1 line
gitgitgadget
PATCH v2 3/5 MyFirstContribution: reference "The cover letter" in "Preparing Email"
In reply to Philippe Blain via GitGitGadgetFrom: Philippe Blain <[email protected]>
The previous commit added a standalone section on the purpose of the cover letter, drawing inspiration from the existing content of the "Preparing Email" section.
Adjust "Preparing Email" to reference "The cover letter", to avoid content duplication.
Also, use the imperative mode for the cover letter subject, as is done in "The cover letter".
--- Documentation/MyFirstContribution.txt | 38 +++++++-------------------- 1 file changed, 9 insertions(+), 29 deletions(-)
Documentation/MyFirstContribution.txt +9 -29
@@ -1040,46 +1040,26 @@ directory you specified - you're nearly ready to send out your review! [[preparing-cover-letter]] === Preparing Email -In addition to an email per patch, the Git community also expects your patches -to come with a cover letter, typically with a subject line [PATCH 0/x] (where -x is the number of patches you're sending). Since you invoked `format-patch` -with `--cover-letter`, you've already got a template ready. Open it up in your -favorite editor. +Since you invoked `format-patch` with `--cover-letter`, you've already got a +cover letter template ready. Open it up in your favorite editor. You should see a number of headers present already. Check that your `From:` -header is correct. Then modify your `Subject:` to something which succinctly -covers the purpose of your entire topic branch, for example: +header is correct. Then modify your `Subject:` (see <<cover-letter,above>> for +how to choose good title for your patch series): ---- -Subject: [PATCH 0/7] adding the 'psuh' command +Subject: [PATCH 0/7] Add the 'psuh' command ---- Make sure you retain the ``[PATCH 0/X]'' part; that's what indicates to the Git -community that this email is the beginning of a review, and many reviewers -filter their email for this type of flag. +community that this email is the beginning of a patch series, and many +reviewers filter their email for this type of flag. You'll need to add some extra parameters when you invoke `git send-email` to add the cover letter. -Next you'll have to fill out the body of your cover letter. This is an important -component of change submission as it explains to the community from a high level -what you're trying to do, and why, in a way that's more apparent than just -looking at your diff. Be sure to explain anything your diff doesn't make clear -on its own. - -Here's an example body for `psuh`: - ----- -Our internal metrics indicate widespread interest in the command -git-psuh - that is, many users are trying to use it, but finding it is -unavailable, using some unknown workaround instead. - -The following handful of patches add the psuh command and implement some -handy features on top of it. - -This patchset is part of the MyFirstContribution tutorial and should not -be merged. ----- +Next you'll have to fill out the body of your cover letter. Again, see +<<cover-letter,above>> for what content to include. The template created by `git format-patch --cover-letter` includes a diffstat. This gives reviewers a summary of what they're in for when reviewing your topic.
Signature · 1 line
gitgitgadget
PATCH v2 5/5 MyFirstContribution: drop PR description for GGG single-patch contributions
In reply to Philippe Blain via GitGitGadgetFrom: Philippe Blain <[email protected]>
By default, GitHub prefills the PR description using the commit message for single-commit PRs. This results in a duplicate commit message below the three-dash line if the contributor does not empty out the PR description before submitting, which adds noise for reviewers.
Add a note to that effect in MyFirstContribution.txt.
This partly addresses:
https://github.com/gitgitgadget/gitgitgadget/issues/340
--- Documentation/MyFirstContribution.txt | 10 ++++++++++ 1 file changed, 10 insertions(+)
Documentation/MyFirstContribution.txt +10 -0
@@ -898,6 +898,16 @@ respectively as the subject and body of the cover letter for your change. Refer to <<cover-letter,"The cover letter">> above for advice on how to title your submission and what content to include in the description. +NOTE: For single-patch contributions, your commit message should already be +meaningful and explain at a high level the purpose (what is happening and why) +of your patch, so you usually do not need any additional context. In that case, +remove the PR description that GitHub automatically generates from your commit +message (your PR description should be empty). If you do need to supply even +more context, you can do so in that space and it will be appended to the email +that GitGitGadget will send, between the three-dash line and the diffstat +(see <<single-patch,Bonus Chapter: One-Patch Changes>> for how this looks once +submitted). + When you're happy, submit your pull request. [[run-ci-ggg]]
Signature · 1 line
gitgitgadget
PATCH v2 4/5 MyFirstContribution: reference "The cover letter" in GitGitGadget section
In reply to Philippe Blain via GitGitGadgetFrom: Philippe Blain <[email protected]>
The "Sending Patches via GitGitGadget" section mentions that the PR title and description will be used as the cover letter, but does not explain what is a cover letter or what should be included in it.
Refer readers to the new "The cover letter" section added in a previous commit.
--- Documentation/MyFirstContribution.txt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
Documentation/MyFirstContribution.txt +6 -2
@@ -893,8 +893,12 @@ https://github.com/gitgitgadget/git and open a PR either with the "New pull request" button or the convenient "Compare & pull request" button that may appear with the name of your newly pushed branch. -Review the PR's title and description, as it's used by GitGitGadget as the cover -letter for your change. When you're happy, submit your pull request. +Review the PR's title and description, as they're used by GitGitGadget +respectively as the subject and body of the cover letter for your change. Refer +to <<cover-letter,"The cover letter">> above for advice on how to title your +submission and what content to include in the description. + +When you're happy, submit your pull request. [[run-ci-ggg]] === Running CI and Getting Ready to Send
Signature · 1 line
gitgitgadget
Re: [PATCH v2 1/5] MyFirstContribution: add "Anatomy of a Patch Series" section
In reply to Philippe Blain via GitGitGadgetQuoted 4 lines · Philippe Blain via GitGitGadget
Documentation/MyFirstContribution.txt +3
-0+We can note a few things: + +- Each commit is sent as a separate email, with the commit message title as
Documentation/MyFirstContribution.txt dependencies. `prove` also makes the output nicer. · line 768
Quoted 10 lines
Documentation/MyFirstContribution.txt +9
-0+ subject, prefixed with "[PATCH _i_/_n_]" for the _i_-th commit of an + _n_-commit series. +- Each patch is sent as a reply to an introductory email called the _cover + letter_ of the series, prefixed "[PATCH 0/_n_]". +- Subsequent iterations of the patch series are labelled "[PATCH v2]", "[PATCH + v3]", etc. and sent with a new cover letter, itself a reply to the cover + letter of the previous iteration (more on that below). + +At this point the tutorial diverges, in order to demonstrate two different methods of formatting your patchset and getting it reviewed.
In case of single-patch series submissions, the anatomy is simple: first the email subject is commit message title prefixed with "[PATCH]" or "[PATCH v_n_]" (in case of n-th iteration), then commit message and the actual diff.
Regarding n-th iteration series, sometimes it is desirable to break the threading so that the original thread (of previous iterations) doesn't get too long, by sending the series as completely new thread. Some contributors (including myself) prefer that way. In that case, the link to previous iteration is provided to aid reviewers.
Signature · 1 line
An old man doll... just what I always wanted! - Clara
Re: [PATCH v2 2/5] MyFirstContribution: add standalone section on cover letter
In reply to Philippe Blain via GitGitGadgetDocumentation/MyFirstContribution.txt We can note a few things: · line 788
Quoted 5 lines · Philippe Blain via GitGitGadget
Documentation/MyFirstContribution.txt +5
-0+The body of the cover letter is used to give additional context to reviewers. +Be sure to explain anything your patches don't make clear on their own, but +remember that since the cover letter is not recorded in the commit history, +anything that might be useful to future readers of the repository's history +should be in your commit messages, not in your cover letter.
I agree with only a half of the last sentence.
Things that are useful for "git log" readers should be in the commit message (but that goes without saying---by definition "git log" readers are reading commit messages). If that material helps to understand the overall topic structure by the reviewers, it is not wrong to have that _also_ in your cover letter. IOW, I sense that "not in your cover letter" is a bit too strong.
Other than that, I found that 1/5 and 2/5 are very nicely written.
Thanks.
Quoted 1 line · Philippe Blain via GitGitGadget
Thanks a lot for the reviews! Here is an updated version.
Thanks a lot for the patches. I made a small comment or two, but other than those, everything looked good.
Will queue.
Quoted 10 lines · Philippe Blain via GitGitGadget
Thanks a lot for the reviews! Here is an updated version.
Changes since v1:
* Based on v1 feedback, instead of reusing the 'git send-email' content verbatim in the GGG section, added a new section and reference it in both 'git send-email' and GGG sections. (patches 1/5-4/5)
* In patch 5/5 (patch 3/3 in v1), tweak wording and add a reference to the "Bonus Chapter: One-Patch Changes" section.
This version addresses all of my earlier comments, and overall looks good to me. Thanks again for updating this documentation!
Quoted 188 lines
v1: Two small improvements to the MyFirstContribution tutorial:
* Describe the purpose of the cover letter in that section also, and give an example just as in the 'git send-email' section
* Instruct contributors to drop the GitHub-generated PR description for single patch contributions.Philippe Blain (5):
MyFirstContribution: add "Anatomy of a Patch Series" section
MyFirstContribution: add standalone section on cover letter
MyFirstContribution: reference "The cover letter" in "Preparing Email"
MyFirstContribution: reference "The cover letter" in GitGitGadget
section
MyFirstContribution: drop PR description for GGG single-patch
contributionsDocumentation/MyFirstContribution.txt | 149 ++++++++++++++++++++------ 1 file changed, 114 insertions(+), 35 deletions(-)base-commit:
6cd33dceed60949e2dbc32e3f0f5e67c4c882e1e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1226%2Fphil-blain%2Fmyfirst-contrib-single-patch-ggg-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1226/phil-blain/myfirst-contrib-single-patch-ggg-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1226Range-diff vs v1:
-: ----------- > 1: 59af7e5e5ad MyFirstContribution: add "Anatomy of a Patch Series" section -: ----------- > 2: 9552d80a80d MyFirstContribution: add standalone section on cover letter 1: 7e84d5b286d ! 3: d7699289ea6 MyFirstContribution: move cover letter description to a separate file@@ Metadata
Author: Philippe Blain <[email protected]>## Commit message ##
- MyFirstContribution: move cover letter description to a separate file
+ MyFirstContribution: reference "The cover letter" in "Preparing Email"- In a subsequent commit we want to reuse the explanation of the purpose of
- The cover letter form the "Sending Patches with git send-email" section
- in the "Sending Patches via GitGitGadget" section.
+ The previous commit added a standalone section on the purpose of the
+ cover letter, drawing inspiration from the existing content of the
+ "Preparing Email" section.- To avoid text duplication, move this explanation to a separate file and
- include it in MyFirstContribution.txt.
+ Adjust "Preparing Email" to reference "The cover letter", to avoid
+ content duplication.- Signed-off-by: Philippe Blain <[email protected]>
+ Also, use the imperative mode for the cover letter subject, as is done
+ in "The cover letter".- ##
Documentation/MyFirstContribution-coverletter.txt(new) ## -@@
-+This is an important component of change submission as it explains to the -+community from a high level what you're trying to do, and why, in a way that's -+more apparent than just looking at your diff. Be sure to explain anything your -+diff doesn't make clear on its own.
-+
-+Here's an example body forpsuh:
-+
-+----
-+Our internal metrics indicate widespread interest in the command -+git-psuh - that is, many users are trying to use it, but finding it is -+unavailable, using some unknown workaround instead.
-+
-+The following handful of patches add the psuh command and implement some -+handy features on top of it.
-+
-+This patchset is part of the MyFirstContribution tutorial and should not -+be merged.
-+----
+ Signed-off-by: Philippe Blain <[email protected]>##
Documentation/MyFirstContribution.txt##
-@@Documentation/MyFirstContribution.txt: filter their email for this type of flag.
+@@Documentation/MyFirstContribution.txt: directory you specified - you're nearly ready to send out your review!
+ [[preparing-cover-letter]]
+ === Preparing Email
+
+-In addition to an email per patch, the Git community also expects your patches
+-to come with a cover letter, typically with a subject line [PATCH 0/x] (where
+-x is the number of patches you're sending). Since you invokedformat-patch
+-with--cover-letter, you've already got a template ready. Open it up in your
+-favorite editor.
++Since you invokedformat-patchwith--cover-letter, you've already got a
++cover letter template ready. Open it up in your favorite editor.
+
+ You should see a number of headers present already. Check that yourFrom:
+-header is correct. Then modify yourSubject:to something which succinctly
+-covers the purpose of your entire topic branch, for example:
++header is correct. Then modify yourSubject:(see <<cover-letter,above>> for
++how to choose good title for your patch series):
+
+ ----
+-Subject: [PATCH 0/7] adding the 'psuh' command
++Subject: [PATCH 0/7] Add the 'psuh' command
+ ----
+
+ Make sure you retain the ``[PATCH 0/X]'' part; that's what indicates to the Git
+-community that this email is the beginning of a review, and many reviewers
+-filter their email for this type of flag.
++community that this email is the beginning of a patch series, and many
++reviewers filter their email for this type of flag.
+
You'll need to add some extra parameters when you invokegit send-emailto add
the cover letter.@@
Documentation/MyFirstContribution.txt: filter their email for this type of flag.
-This patchset is part of the MyFirstContribution tutorial and should not -be merged.
-----
-+Next you'll have to fill out the body of your cover letter. -+include::MyFirstContribution-coverletter.txt[]
++Next you'll have to fill out the body of your cover letter. Again, see ++<<cover-letter,above>> for what content to include.The template created by
git format-patch --cover-letterincludes a diffstat. This gives reviewers a summary of what they're in for when reviewing your topic.
2:afb80b8e9ee! 4:f6034b0964bMyFirstContribution: also explain cover letter in GitGitGadget section
@@ Metadata
Author: Philippe Blain <[email protected]>## Commit message ##
- MyFirstContribution: also explain cover letter in GitGitGadget section
+ MyFirstContribution: reference "The cover letter" in GitGitGadget sectionThe "Sending Patches via GitGitGadget" section mentions that the PR title and description will be used as the cover letter, but does not explain what is a cover letter or what should be included in it.
- Mention the purpose of the cover letter in that section, and give
- examples for the title and description, leveraging the excerpt extracted
- from the "Sending Patches with git send-email" section in the previous
+ Refer readers to the new "The cover letter" section added in a previous
commit.Signed-off-by Philippe Blain <[email protected]>@@
Documentation/MyFirstContribution.txt: https://github.com/gitgitgadget/git and o-Review the PR's title and description, as it's used by GitGitGadget as the cover -letter for your change. When you're happy, submit your pull request.
-+Review the PR's title and description, as they're used by GitGitGadget as the -+cover letter for your change. The cover letter describes your proposed -+contribution as a whole and is ideal to mention any context that might be -+useful for reviewers. The title of your PR should be something which -+succinctly covers the purpose of your entire topic branch, for example:
-+
-+----
-+Adding the 'psuh' command
-+----
-+
-+Your PR's description will used as the body of the cover letter. -+include::MyFirstContribution-coverletter.txt[]
++Review the PR's title and description, as they're used by GitGitGadget ++respectively as the subject and body of the cover letter for your change. Refer ++to <<cover-letter,"The cover letter">> above for advice on how to title your ++submission and what content to include in the description.
+
+When you're happy, submit your pull request.3:
2f6ecbf2601! 5:33256c6b4baMyFirstContribution: drop PR description for GGG single-patch contributions
@@ Commit messageSigned-off-by Philippe Blain <[email protected]>##
Documentation/MyFirstContribution.txt##
-@@Documentation/MyFirstContribution.txt: Adding the 'psuh' command
- Your PR's description will used as the body of the cover letter.
- include::MyFirstContribution-coverletter.txt[]
+@@Documentation/MyFirstContribution.txt: respectively as the subject and body of the cover letter for your change. Refer
+ to <<cover-letter,"The cover letter">> above for advice on how to title your
+ submission and what content to include in the description.+NOTE: For single-patch contributions, your commit message should already be +meaningful and explain at a high level the purpose (what is happening and why)
@@Documentation/MyFirstContribution.txt: Adding the 'psuh' command
+remove the PR description that GitHub automatically generates from your commit +message (your PR description should be empty). If you do need to supply even +more context, you can do so in that space and it will be appended to the email
-+that GitGitGadget will send, separately from your commit message. ++that GitGitGadget will send, between the three-dash line and the diffstat ++(see <<single-patch,Bonus Chapter: One-Patch Changes>> for how this looks once ++submitted).
+
When you're happy, submit your pull request.
Re: [PATCH v2 1/5] MyFirstContribution: add "Anatomy of a Patch Series" section
In reply to Bagas SanjayaQuoted 12 lines · Bagas Sanjaya
Quoted 11 lines · Philippe Blain via GitGitGadget
Documentation/MyFirstContribution.txt +11
-0+We can note a few things: + +- Each commit is sent as a separate email, with the commit message title as + subject, prefixed with "[PATCH _i_/_n_]" for the _i_-th commit of an + _n_-commit series. +- Each patch is sent as a reply to an introductory email called the _cover + letter_ of the series, prefixed "[PATCH 0/_n_]". +- Subsequent iterations of the patch series are labelled "[PATCH v2]", "[PATCH + v3]", etc. and sent with a new cover letter, itself a reply to the cover + letter of the previous iteration (more on that below). +
Quoted 8 lines
Quoted 3 lines
+At this point the tutorial diverges, in order to demonstrate two
different methods of formatting your patchset and getting it reviewed.In case of single-patch series submissions, the anatomy is simple: first the email subject is commit message title prefixed with "[PATCH]" or "[PATCH
v_n_]" (in case of n-th iteration), then commit message and the actual diff.
Correct. There is no single-patch topic in the summary view shown in the document, so it does not belong to the above "We can note a few things" list. But I agree that there should be a mention for a single-patch topic somewhere in this document, both for a patch and (the usual lack of) cover-letter for such a topic.
The sample topic this tutorial uses is a multi-patch series and everything in the document revolves around handing a multi-patch series, so finding a good place to fit it may be a bit tricky, though.
Quoted 5 lines
Regarding n-th iteration series, sometimes it is desirable to break the threading so that the original thread (of previous iterations) doesn't get too long, by sending the series as completely new thread. Some contributors (including myself) prefer that way. In that case, the link to previous iteration is provided to aid reviewers.
This is often not very friendly to reviewers, unless the "new topic" is so different that it is almost totally unrelated to the old one.
Even in a "in earlier round, we perceived X as a problem and tried to solve it, but it turns out it is better to solve Y instead" case, it often helps to learn the reason why we ended up not addressing X after you use "git blame" to find a commit that solved Y in a later iteration and wonder why an approach to solve X was not taken. It is a very good idea to mention the need to add a link to a previous thread if the submitter decides to break the thread.
Thanks.
Re: [PATCH v2 1/5] MyFirstContribution: add "Anatomy of a Patch Series" section
Hi Bagas and Junio,
Le 2022-05-11 à 17:30, Junio C Hamano a écrit :
Quoted 29 lines
Quoted 12 lines · Bagas Sanjaya
Quoted 11 lines · Philippe Blain via GitGitGadget
Documentation/MyFirstContribution.txt +11
-0+We can note a few things: + +- Each commit is sent as a separate email, with the commit message title as + subject, prefixed with "[PATCH _i_/_n_]" for the _i_-th commit of an + _n_-commit series. +- Each patch is sent as a reply to an introductory email called the _cover + letter_ of the series, prefixed "[PATCH 0/_n_]". +- Subsequent iterations of the patch series are labelled "[PATCH v2]", "[PATCH + v3]", etc. and sent with a new cover letter, itself a reply to the cover + letter of the previous iteration (more on that below). +Quoted 8 lines
Quoted 3 lines
+At this point the tutorial diverges, in order to demonstrate two
different methods of formatting your patchset and getting it reviewed.In case of single-patch series submissions, the anatomy is simple: first the email subject is commit message title prefixed with "[PATCH]" or "[PATCH
v_n_]" (in case of n-th iteration), then commit message and the actual diff.Correct. There is no single-patch topic in the summary view shown in the document, so it does not belong to the above "We can note a few things" list. But I agree that there should be a mention for a single-patch topic somewhere in this document, both for a patch and (the usual lack of) cover-letter for such a topic.
Quoted 4 lines
The sample topic this tutorial uses is a multi-patch series and everything in the document revolves around handing a multi-patch series, so finding a good place to fit it may be a bit tricky, though.
There is already a small section on single-patch topics, at the end of the 'git send-email' section [1]. And I add a link to it in the GGG section in patch 5/5.
[1] https://git-scm.com/docs/MyFirstContribution#single-patch
Quoted 9 lines
Quoted 5 lines
Regarding n-th iteration series, sometimes it is desirable to break the threading so that the original thread (of previous iterations) doesn't get too long, by sending the series as completely new thread. Some contributors (including myself) prefer that way. In that case, the link to previous iteration is provided to aid reviewers.
This is often not very friendly to reviewers, unless the "new topic" is so different that it is almost totally unrelated to the old one.
Quoted 7 lines
Even in a "in earlier round, we perceived X as a problem and tried to solve it, but it turns out it is better to solve Y instead" case, it often helps to learn the reason why we ended up not addressing X after you use "git blame" to find a commit that solved Y in a later iteration and wonder why an approach to solve X was not taken. It is a very good idea to mention the need to add a link to a previous thread if the submitter decides to break the thread.
Thanks for confirming. I think for a beginner tutorial like MyFirstContribution, we should keep things simple, so I would leave the content as-is regarding threading. If we ever revive the "mailing list etiquette" series, then I think we could add notes regarding this kind of things.
Cheers,
Philippe.
Re: [PATCH v2 2/5] MyFirstContribution: add standalone section on cover letter
In reply to Junio C HamanoHi Junio,
Le 2022-05-11 à 06:01, Junio C Hamano a écrit :
Quoted 10 lines
Documentation/MyFirstContribution.txt We can note a few things: · line 788
Quoted 5 lines · Philippe Blain via GitGitGadget
Documentation/MyFirstContribution.txt +5
-0+The body of the cover letter is used to give additional context to reviewers. +Be sure to explain anything your patches don't make clear on their own, but +remember that since the cover letter is not recorded in the commit history, +anything that might be useful to future readers of the repository's history +should be in your commit messages, not in your cover letter.I agree with only a half of the last sentence.
Quoted 6 lines
Things that are useful for "git log" readers should be in the commit message (but that goes without saying---by definition "git log" readers are reading commit messages). If that material helps to understand the overall topic structure by the reviewers, it is not wrong to have that _also_ in your cover letter. IOW, I sense that "not in your cover letter" is a bit too strong.
OK, I'll tweak that in a v3.
Quoted 2 lines
Other than that, I found that 1/5 and 2/5 are very nicely written.
Thanks !:)
Re: [PATCH v2 1/5] MyFirstContribution: add "Anatomy of a Patch Series" section
In reply to Philippe BlainQuoted 34 lines · Philippe Blain
Quoted 29 lines
Quoted 12 lines · Bagas Sanjaya
Quoted 11 lines · Philippe Blain via GitGitGadget
Documentation/MyFirstContribution.txt +11
-0+We can note a few things: + +- Each commit is sent as a separate email, with the commit message title as + subject, prefixed with "[PATCH _i_/_n_]" for the _i_-th commit of an + _n_-commit series. +- Each patch is sent as a reply to an introductory email called the _cover + letter_ of the series, prefixed "[PATCH 0/_n_]". +- Subsequent iterations of the patch series are labelled "[PATCH v2]", "[PATCH + v3]", etc. and sent with a new cover letter, itself a reply to the cover + letter of the previous iteration (more on that below). +Quoted 8 lines
Quoted 3 lines
+At this point the tutorial diverges, in order to demonstrate two
different methods of formatting your patchset and getting it reviewed.In case of single-patch series submissions, the anatomy is simple: first the email subject is commit message title prefixed with "[PATCH]" or "[PATCH
v_n_]" (in case of n-th iteration), then commit message and the actual diff.Correct. There is no single-patch topic in the summary view shown in the document, so it does not belong to the above "We can note a few things" list. But I agree that there should be a mention for a single-patch topic somewhere in this document, both for a patch and (the usual lack of) cover-letter for such a topic.
Quoted 4 lines
The sample topic this tutorial uses is a multi-patch series and everything in the document revolves around handing a multi-patch series, so finding a good place to fit it may be a bit tricky, though.
Quoted 5 lines
There is already a small section on single-patch topics, at the end of the 'git send-email' section [1]. And I add a link to it in the GGG section in patch 5/5.
[1] https://git-scm.com/docs/MyFirstContribution#single-patch
Yup, I was wondering if there is an easy way to move it to this section, which imparts a knowledge common across different method of patch submission. If we can do so somewhere nearby, it would be ideal. Perhaps in "We can note a few things" list or as a side note immediately after the list, we can add
- A single-patch topic is sent with "[PATCH]", "[PATCH v2]", etc. without _i_/_n_ numbering (in the above thread overview, no single-patch topic appears, though).
Or we can redo the lore screenshot to include such a topic, perhaps, then we can lose the comment in the parentheses.
That reminds me of one small bug in your patch, where it says
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Documentation/MyFirstContribution.txt dependencies. `prove` also makes the output nicer. · line 765
Quoted 3 lines
+- Subsequent iterations of the patch series are labelled "[PATCH v2]", "[PATCH
Documentation/MyFirstContribution.txt +2
-0+ v3]", etc. and sent with a new cover letter, itself a reply to the cover + letter of the previous iteration (more on that below).
Subsequent interations of the patches and the cover letter use "PATCH v2", "PATCH v3", etc. in place of "PATCH". "[PATCH v2 1/3]" would be the first of three patches in the second iteration, for example, and replies to "[PATCH v2 0/3]" which is the cover letter for the iteration, which in turn replies to the cover letter of the first iteration.
Thanks.
Re: [PATCH v2 1/5] MyFirstContribution: add "Anatomy of a Patch Series" section
In reply to Philippe Blain via GitGitGadgetDocumentation/MyFirstContribution.txt dependencies. `prove` also makes the output nicer. · line 713
Quoted 10 lines · Philippe Blain via GitGitGadget
Documentation/MyFirstContribution.txt +1
-1@@ -710,13 +710,61 @@ dependencies. `prove` also makes the output nicer. Go ahead and commit this change, as well. [[ready-to-share]] -== Getting Ready to Share +== Getting Ready to Share: Anatomy of a Patch Series
Shouldn't the title also include the magic word 'Contribution'? Otherwise the change below may look inconsistent.
Documentation/MyFirstContribution.txt dependencies. `prove` also makes the output nicer. · line 717
Quoted 4 lines
Documentation/MyFirstContribution.txt +1
-1You may have noticed already that the Git project performs its code reviews via emailed patches, which are then applied by the maintainer when they are ready -and approved by the community. The Git project does not accept patches from +and approved by the community. The Git project does not accept contributions from
--
Philip
Re: [PATCH v2 1/5] MyFirstContribution: add "Anatomy of a Patch Series" section
Hi Philip,
Le 2022-05-12 à 08:11, Philip Oakley a écrit :
Quoted 11 lines · Philippe Blain via GitGitGadget
Quoted 9 lines
Documentation/MyFirstContribution.txt +0
-0@@ -710,13 +710,61 @@ dependencies. `prove` also makes the output nicer.Go ahead and commit this change, as well. [[ready-to-share]]
-== Getting Ready to Share
+== Getting Ready to Share: Anatomy of a Patch SeriesShouldn't the title also include the magic word 'Contribution'? Otherwise the change below may look inconsistent.
I don't think so. This section is to introduce "what is a patch series"
to readers that might be reading the word "patch" in a software development context for the very first time. Patch series are how Git is developped, so the title here should include these words, I think.
Quoted 5 lines
Documentation/MyFirstContribution.txt dependencies. `prove` also makes the output nicer. · line 717
Quoted 5 lines
You may have noticed already that the Git project performs its code reviews via emailed patches, which are then applied by the maintainer when they are ready
Documentation/MyFirstContribution.txt +1
-1-and approved by the community. The Git project does not accept patches from +and approved by the community. The Git project does not accept contributions from
the next two words of this sentence are "from pull requests". People that are familiar with GitHub will probably have heard "contributions" in the context of GitHub "pull requests", but maybe not "patches". So that's why I changed the wording here.
Thank you for your comments,
Philippe.
Re: [PATCH v2 1/5] MyFirstContribution: add "Anatomy of a Patch Series" section
In reply to Junio C HamanoHi Junio,
Le 2022-05-11 à 18:09, Junio C Hamano a écrit :
Quoted 48 lines
Quoted 34 lines · Philippe Blain
Quoted 29 lines
Quoted 12 lines · Bagas Sanjaya
Quoted 11 lines · Philippe Blain via GitGitGadget
Documentation/MyFirstContribution.txt +11
-0+We can note a few things: + +- Each commit is sent as a separate email, with the commit message title as + subject, prefixed with "[PATCH _i_/_n_]" for the _i_-th commit of an + _n_-commit series. +- Each patch is sent as a reply to an introductory email called the _cover + letter_ of the series, prefixed "[PATCH 0/_n_]". +- Subsequent iterations of the patch series are labelled "[PATCH v2]", "[PATCH + v3]", etc. and sent with a new cover letter, itself a reply to the cover + letter of the previous iteration (more on that below). +Quoted 8 lines
Quoted 3 lines
+At this point the tutorial diverges, in order to demonstrate two
different methods of formatting your patchset and getting it reviewed.In case of single-patch series submissions, the anatomy is simple: first the email subject is commit message title prefixed with "[PATCH]" or "[PATCH
v_n_]" (in case of n-th iteration), then commit message and the actual diff.Correct. There is no single-patch topic in the summary view shown in the document, so it does not belong to the above "We can note a few things" list. But I agree that there should be a mention for a single-patch topic somewhere in this document, both for a patch and (the usual lack of) cover-letter for such a topic.
Quoted 4 lines
The sample topic this tutorial uses is a multi-patch series and everything in the document revolves around handing a multi-patch series, so finding a good place to fit it may be a bit tricky, though.
Quoted 5 lines
There is already a small section on single-patch topics, at the end of the 'git send-email' section [1]. And I add a link to it in the GGG section in patch 5/5.
[1] https://git-scm.com/docs/MyFirstContribution#single-patch
Yup, I was wondering if there is an easy way to move it to this section, which imparts a knowledge common across different method of patch submission. If we can do so somewhere nearby, it would be ideal. Perhaps in "We can note a few things" list or as a side note immediately after the list, we can add
Quoted 6 lines
- A single-patch topic is sent with "[PATCH]", "[PATCH v2]", etc. without _i_/_n_ numbering (in the above thread overview, no single-patch topic appears, though).
Or we can redo the lore screenshot to include such a topic, perhaps, then we can lose the comment in the parentheses.
OK, a little side note sounds good.
Quoted 6 lines
That reminds me of one small bug in your patch, where it says
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Documentation/MyFirstContribution.txt dependencies. `prove` also makes the output nicer. · line 765
Quoted 3 lines
+- Subsequent iterations of the patch series are labelled "[PATCH v2]", "[PATCH
Documentation/MyFirstContribution.txt +2
-0+ v3]", etc. and sent with a new cover letter, itself a reply to the cover + letter of the previous iteration (more on that below).
Quoted 6 lines
Subsequent interations of the patches and the cover letter use "PATCH v2", "PATCH v3", etc. in place of "PATCH". "[PATCH v2 1/3]" would be the first of three patches in the second iteration, for example, and replies to "[PATCH v2 0/3]" which is the cover letter for the iteration, which in turn replies to the cover letter of the first iteration.
OK, I will add more details along these lines.
v3 cover Improve MyFirstContribution's GitGitGadget section
In reply to Philippe Blain via GitGitGadgetChanges since v2:
* tweaked 2/5 following Junio's comments (added more details, tweaked the wording and added a note about single patch series).
v2: Thanks a lot for the reviews! Here is an updated version.
Changes since v1:
* Based on v1 feedback, instead of reusing the 'git send-email' content verbatim in the GGG section, added a new section and reference it in both 'git send-email' and GGG sections. (patches 1/5-4/5)
* In patch 5/5 (patch 3/3 in v1), tweak wording and add a reference to the "Bonus Chapter: One-Patch Changes" section.
v1: Two small improvements to the MyFirstContribution tutorial:
* Describe the purpose of the cover letter in that section also, and give an example just as in the 'git send-email' section
* Instruct contributors to drop the GitHub-generated PR description for single patch contributions.
Philippe Blain (5):
MyFirstContribution: add "Anatomy of a Patch Series" section
MyFirstContribution: add standalone section on cover letter
MyFirstContribution: reference "The cover letter" in "Preparing Email"
MyFirstContribution: reference "The cover letter" in GitGitGadget
section
MyFirstContribution: drop PR description for GGG single-patch
contributions
Documentation/MyFirstContribution.txt | 155 ++++++++++++++++++++------ 1 file changed, 120 insertions(+), 35 deletions(-)
base-commit: 6cd33dceed60949e2dbc32e3f0f5e67c4c882e1e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1226%2Fphil-blain%2Fmyfirst-contrib-single-patch-ggg-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1226/phil-blain/myfirst-contrib-single-patch-ggg-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1226
Range-diff vs v2:
1: 59af7e5e5ad = 1: 59af7e5e5ad MyFirstContribution: add "Anatomy of a Patch Series" section 2: 9552d80a80d ! 2: ee3238f5a91 MyFirstContribution: add standalone section on cover letter
@@ Commit message
## Documentation/MyFirstContribution.txt ##
@@ Documentation/MyFirstContribution.txt: We can note a few things:
- v3]", etc. and sent with a new cover letter, itself a reply to the cover
- letter of the previous iteration (more on that below).
-
+ _n_-commit series.
+ - Each patch is sent as a reply to an introductory email called the _cover
+ letter_ of the series, prefixed "[PATCH 0/_n_]".
+-- Subsequent iterations of the patch series are labelled "[PATCH v2]", "[PATCH
+- v3]", etc. and sent with a new cover letter, itself a reply to the cover
+- letter of the previous iteration (more on that below).
++- Subsequent iterations of the patch series are labelled "PATCH v2", "PATCH
++ v3", etc. in place of "PATCH". For example, "[PATCH v2 1/3]" would be the first of
++ three patches in the second iteration. Each iteration is sent with a new cover
++ letter (like "[PATCH v2 0/3]" above), itself a reply to the cover letter of the
++ previous iteration (more on that below).
++
++NOTE: A single-patch topic is sent with "[PATCH]", "[PATCH v2]", etc. without
++_i_/_n_ numbering (in the above thread overview, no single-patch topic appears,
++though).
++
+[[cover-letter]]
+=== The cover letter
+
@@ Documentation/MyFirstContribution.txt: We can note a few things:
+Be sure to explain anything your patches don't make clear on their own, but
+remember that since the cover letter is not recorded in the commit history,
+anything that might be useful to future readers of the repository's history
-+should be in your commit messages, not in your cover letter.
++should also be in your commit messages.
+
+Here's an example body for psuh:
+
@@ Documentation/MyFirstContribution.txt: We can note a few things:
+This patchset is part of the MyFirstContribution tutorial and should not
+be merged.
+----
-+
+
At this point the tutorial diverges, in order to demonstrate two
different methods of formatting your patchset and getting it reviewed.
-
@@ Documentation/MyFirstContribution.txt: but want reviewers to look at what they have so far. You can add this flag with
Check and make sure that your patches and cover letter template exist in the
directory you specified - you're nearly ready to send out your review!
3: d7699289ea6 = 3: 2caec6e619a MyFirstContribution: reference "The cover letter" in "Preparing Email" 4: f6034b0964b = 4: 02598d431e0 MyFirstContribution: reference "The cover letter" in GitGitGadget section 5: 33256c6b4ba = 5: 76bcb6612e4 MyFirstContribution: drop PR description for GGG single-patch contributions
Signature · 1 line
gitgitgadget
PATCH v3 3/5 MyFirstContribution: reference "The cover letter" in "Preparing Email"
From: Philippe Blain <[email protected]>
The previous commit added a standalone section on the purpose of the cover letter, drawing inspiration from the existing content of the "Preparing Email" section.
Adjust "Preparing Email" to reference "The cover letter", to avoid content duplication.
Also, use the imperative mode for the cover letter subject, as is done in "The cover letter".
--- Documentation/MyFirstContribution.txt | 38 +++++++-------------------- 1 file changed, 9 insertions(+), 29 deletions(-)
Documentation/MyFirstContribution.txt +9 -29
@@ -1046,46 +1046,26 @@ directory you specified - you're nearly ready to send out your review! [[preparing-cover-letter]] === Preparing Email -In addition to an email per patch, the Git community also expects your patches -to come with a cover letter, typically with a subject line [PATCH 0/x] (where -x is the number of patches you're sending). Since you invoked `format-patch` -with `--cover-letter`, you've already got a template ready. Open it up in your -favorite editor. +Since you invoked `format-patch` with `--cover-letter`, you've already got a +cover letter template ready. Open it up in your favorite editor. You should see a number of headers present already. Check that your `From:` -header is correct. Then modify your `Subject:` to something which succinctly -covers the purpose of your entire topic branch, for example: +header is correct. Then modify your `Subject:` (see <<cover-letter,above>> for +how to choose good title for your patch series): ---- -Subject: [PATCH 0/7] adding the 'psuh' command +Subject: [PATCH 0/7] Add the 'psuh' command ---- Make sure you retain the ``[PATCH 0/X]'' part; that's what indicates to the Git -community that this email is the beginning of a review, and many reviewers -filter their email for this type of flag. +community that this email is the beginning of a patch series, and many +reviewers filter their email for this type of flag. You'll need to add some extra parameters when you invoke `git send-email` to add the cover letter. -Next you'll have to fill out the body of your cover letter. This is an important -component of change submission as it explains to the community from a high level -what you're trying to do, and why, in a way that's more apparent than just -looking at your diff. Be sure to explain anything your diff doesn't make clear -on its own. - -Here's an example body for `psuh`: - ----- -Our internal metrics indicate widespread interest in the command -git-psuh - that is, many users are trying to use it, but finding it is -unavailable, using some unknown workaround instead. - -The following handful of patches add the psuh command and implement some -handy features on top of it. - -This patchset is part of the MyFirstContribution tutorial and should not -be merged. ----- +Next you'll have to fill out the body of your cover letter. Again, see +<<cover-letter,above>> for what content to include. The template created by `git format-patch --cover-letter` includes a diffstat. This gives reviewers a summary of what they're in for when reviewing your topic.
Signature · 1 line
gitgitgadget
PATCH v3 1/5 MyFirstContribution: add "Anatomy of a Patch Series" section
In reply to Philippe Blain via GitGitGadgetFrom: Philippe Blain <[email protected]>
Before describing how to send patches to the mailing list either with GitGitGadget or 'git send-email', the MyFirstContribution tutorial includes a small "Getting Ready to Share" section where the two different methods are briefly introduced.
Use this section to also describe what a patch series looks like once submitted, so that readers get an understanding of the end result before diving into how to accomplish that end result.
Start by copying the "thread overview" section of a recent contribution from the public-inbox web UI and explaining how each commit is a separate mail, and point out the cover letter.
Subsequent commits will move the existing description of the purpose of the cover letter from the 'git send-email' section to this "anatomy" section.
Also, change the wording in the introductory paragraph to use "contributions" instead of "patches", since this makes more sense when talking about GitHub pull requests.
--- Documentation/MyFirstContribution.txt | 54 +++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 3 deletions(-)
Documentation/MyFirstContribution.txt +51 -3
@@ -710,13 +710,61 @@ dependencies. `prove` also makes the output nicer. Go ahead and commit this change, as well. [[ready-to-share]] -== Getting Ready to Share +== Getting Ready to Share: Anatomy of a Patch Series You may have noticed already that the Git project performs its code reviews via emailed patches, which are then applied by the maintainer when they are ready -and approved by the community. The Git project does not accept patches from +and approved by the community. The Git project does not accept contributions from pull requests, and the patches emailed for review need to be formatted a -specific way. At this point the tutorial diverges, in order to demonstrate two +specific way. + +:patch-series: https://lore.kernel.org/git/[email protected]/ +:lore: https://lore.kernel.org/git/ + +Before taking a look at how to convert your commits into emailed patches, +let's analyze what the end result, a "patch series", looks like. Here is an +{patch-series}[example] of the summary view for a patch series on the web interface of +the {lore}[Git mailing list archive]: + +---- +2022-02-18 18:40 [PATCH 0/3] libify reflog John Cai via GitGitGadget +2022-02-18 18:40 ` [PATCH 1/3] reflog: libify delete reflog function and helpers John Cai via GitGitGadget +2022-02-18 19:10 ` Ævar Arnfjörð Bjarmason [this message] +2022-02-18 19:39 ` Taylor Blau +2022-02-18 19:48 ` Ævar Arnfjörð Bjarmason +2022-02-18 19:35 ` Taylor Blau +2022-02-21 1:43 ` John Cai +2022-02-21 1:50 ` Taylor Blau +2022-02-23 19:50 ` John Cai +2022-02-18 20:00 ` // other replies ellided +2022-02-18 18:40 ` [PATCH 2/3] reflog: call reflog_delete from reflog.c John Cai via GitGitGadget +2022-02-18 19:15 ` Ævar Arnfjörð Bjarmason +2022-02-18 20:26 ` Junio C Hamano +2022-02-18 18:40 ` [PATCH 3/3] stash: call reflog_delete from reflog.c John Cai via GitGitGadget +2022-02-18 19:20 ` Ævar Arnfjörð Bjarmason +2022-02-19 0:21 ` Taylor Blau +2022-02-22 2:36 ` John Cai +2022-02-22 10:51 ` Ævar Arnfjörð Bjarmason +2022-02-18 19:29 ` [PATCH 0/3] libify reflog Ævar Arnfjörð Bjarmason +2022-02-22 18:30 ` [PATCH v2 0/3] libify reflog John Cai via GitGitGadget +2022-02-22 18:30 ` [PATCH v2 1/3] stash: add test to ensure reflog --rewrite --updatref behavior John Cai via GitGitGadget +2022-02-23 8:54 ` Ævar Arnfjörð Bjarmason +2022-02-23 21:27 ` Junio C Hamano +// continued +---- + +We can note a few things: + +- Each commit is sent as a separate email, with the commit message title as + subject, prefixed with "[PATCH _i_/_n_]" for the _i_-th commit of an + _n_-commit series. +- Each patch is sent as a reply to an introductory email called the _cover + letter_ of the series, prefixed "[PATCH 0/_n_]". +- Subsequent iterations of the patch series are labelled "[PATCH v2]", "[PATCH + v3]", etc. and sent with a new cover letter, itself a reply to the cover + letter of the previous iteration (more on that below). + +At this point the tutorial diverges, in order to demonstrate two different methods of formatting your patchset and getting it reviewed. The first method to be covered is GitGitGadget, which is useful for those
Signature · 1 line
gitgitgadget
PATCH v3 5/5 MyFirstContribution: drop PR description for GGG single-patch contributions
In reply to Philippe Blain via GitGitGadgetFrom: Philippe Blain <[email protected]>
By default, GitHub prefills the PR description using the commit message for single-commit PRs. This results in a duplicate commit message below the three-dash line if the contributor does not empty out the PR description before submitting, which adds noise for reviewers.
Add a note to that effect in MyFirstContribution.txt.
This partly addresses:
https://github.com/gitgitgadget/gitgitgadget/issues/340
--- Documentation/MyFirstContribution.txt | 10 ++++++++++ 1 file changed, 10 insertions(+)
Documentation/MyFirstContribution.txt +10 -0
@@ -904,6 +904,16 @@ respectively as the subject and body of the cover letter for your change. Refer to <<cover-letter,"The cover letter">> above for advice on how to title your submission and what content to include in the description. +NOTE: For single-patch contributions, your commit message should already be +meaningful and explain at a high level the purpose (what is happening and why) +of your patch, so you usually do not need any additional context. In that case, +remove the PR description that GitHub automatically generates from your commit +message (your PR description should be empty). If you do need to supply even +more context, you can do so in that space and it will be appended to the email +that GitGitGadget will send, between the three-dash line and the diffstat +(see <<single-patch,Bonus Chapter: One-Patch Changes>> for how this looks once +submitted). + When you're happy, submit your pull request. [[run-ci-ggg]]
Signature · 1 line
gitgitgadget
PATCH v3 4/5 MyFirstContribution: reference "The cover letter" in GitGitGadget section
In reply to Philippe Blain via GitGitGadgetFrom: Philippe Blain <[email protected]>
The "Sending Patches via GitGitGadget" section mentions that the PR title and description will be used as the cover letter, but does not explain what is a cover letter or what should be included in it.
Refer readers to the new "The cover letter" section added in a previous commit.
--- Documentation/MyFirstContribution.txt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
Documentation/MyFirstContribution.txt +6 -2
@@ -899,8 +899,12 @@ https://github.com/gitgitgadget/git and open a PR either with the "New pull request" button or the convenient "Compare & pull request" button that may appear with the name of your newly pushed branch. -Review the PR's title and description, as it's used by GitGitGadget as the cover -letter for your change. When you're happy, submit your pull request. +Review the PR's title and description, as they're used by GitGitGadget +respectively as the subject and body of the cover letter for your change. Refer +to <<cover-letter,"The cover letter">> above for advice on how to title your +submission and what content to include in the description. + +When you're happy, submit your pull request. [[run-ci-ggg]] === Running CI and Getting Ready to Send
Signature · 1 line
gitgitgadget
PATCH v3 2/5 MyFirstContribution: add standalone section on cover letter
In reply to Philippe Blain via GitGitGadgetFrom: Philippe Blain <[email protected]>
An explanation of the purpose of the cover letter is included in the "Sending Patches with git send-email" / "Preparing Email" section but is missing from the "Sending Patches via GitGitGadget" section.
Add a standalone section "The cover letter" under the "Getting Started:
Anatomy of a Patch Series" header to explain what the cover letter is used for and to draft the cover letter of the 'psuh' topic used in the tutorial.
For now we mostly copy content from the "Sending Patches with git send-email" section but do not adjust that section, nor the GGG section, to reference the new section. This is done in following commits.
Also, adjust the "Preparing Email" Asciidoc anchor to avoid conflicts.
--- Documentation/MyFirstContribution.txt | 51 ++++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 4 deletions(-)
Documentation/MyFirstContribution.txt +47 -4
@@ -760,9 +760,52 @@ We can note a few things: _n_-commit series. - Each patch is sent as a reply to an introductory email called the _cover letter_ of the series, prefixed "[PATCH 0/_n_]". -- Subsequent iterations of the patch series are labelled "[PATCH v2]", "[PATCH - v3]", etc. and sent with a new cover letter, itself a reply to the cover - letter of the previous iteration (more on that below). +- Subsequent iterations of the patch series are labelled "PATCH v2", "PATCH + v3", etc. in place of "PATCH". For example, "[PATCH v2 1/3]" would be the first of + three patches in the second iteration. Each iteration is sent with a new cover + letter (like "[PATCH v2 0/3]" above), itself a reply to the cover letter of the + previous iteration (more on that below). + +NOTE: A single-patch topic is sent with "[PATCH]", "[PATCH v2]", etc. without +_i_/_n_ numbering (in the above thread overview, no single-patch topic appears, +though). + +[[cover-letter]] +=== The cover letter + +In addition to an email per patch, the Git community also expects your patches +to come with a cover letter. This is an important component of change +submission as it explains to the community from a high level what you're trying +to do, and why, in a way that's more apparent than just looking at your +patches. + +The title of your cover letter should be something which succinctly covers the +purpose of your entire topic branch. It's often in the imperative mood, just +like our commit message titles. Here is how we'll title our series: + +--- +Add the 'psuh' command +--- + +The body of the cover letter is used to give additional context to reviewers. +Be sure to explain anything your patches don't make clear on their own, but +remember that since the cover letter is not recorded in the commit history, +anything that might be useful to future readers of the repository's history +should also be in your commit messages. + +Here's an example body for `psuh`: + +---- +Our internal metrics indicate widespread interest in the command +git-psuh - that is, many users are trying to use it, but finding it is +unavailable, using some unknown workaround instead. + +The following handful of patches add the psuh command and implement some +handy features on top of it. + +This patchset is part of the MyFirstContribution tutorial and should not +be merged. +---- At this point the tutorial diverges, in order to demonstrate two different methods of formatting your patchset and getting it reviewed.
@@ -1000,7 +1043,7 @@ but want reviewers to look at what they have so far. You can add this flag with Check and make sure that your patches and cover letter template exist in the directory you specified - you're nearly ready to send out your review! -[[cover-letter]] +[[preparing-cover-letter]] === Preparing Email In addition to an email per patch, the Git community also expects your patches
Signature · 1 line
gitgitgadget
Quoted 8 lines · Philippe Blain via GitGitGadget
Philippe Blain (5):
MyFirstContribution: add "Anatomy of a Patch Series" section
MyFirstContribution: add standalone section on cover letter
MyFirstContribution: reference "The cover letter" in "Preparing Email"
MyFirstContribution: reference "The cover letter" in GitGitGadget
section
MyFirstContribution: drop PR description for GGG single-patch
contributions
I am planning to merge it to 'next' soonish. Please speak up if anybody has any issues.
Thanks.
Hi Junio,
Le 2022-05-19 à 17:49, Junio C Hamano a écrit :
Quoted 11 lines
Quoted 8 lines · Philippe Blain via GitGitGadget
Philippe Blain (5):
MyFirstContribution: add "Anatomy of a Patch Series" section
MyFirstContribution: add standalone section on cover letter
MyFirstContribution: reference "The cover letter" in "Preparing Email"
MyFirstContribution: reference "The cover letter" in GitGitGadget
section
MyFirstContribution: drop PR description for GGG single-patch
contributions
Quoted 5 lines
I am planning to merge it to 'next' soonish. Please speak up if anybody has any issues.
Thanks.
I just realized that the additional/tweaked content that I added in v3 went into 2/5 where as they should have gone into 1/5 (I'm talking about the changes *before* the "The cover letter" heading in [1]). I slightly messed up my rebase it seems, I'm sorry...
Should I send a new version, or is it too late since it's now in 'next' ?
Thanks,
Philippe.
[1] https://github.com/gitgitgadget/git/pull/1226/commits/ee3238f5a9169f0c7d046089c7951791b8db3cab
Quoted 8 lines · Philippe Blain
Quoted 8 lines
Quoted 8 lines
Philippe Blain (5):
MyFirstContribution: add "Anatomy of a Patch Series" section
MyFirstContribution: add standalone section on cover letter
MyFirstContribution: reference "The cover letter" in "Preparing Email"
MyFirstContribution: reference "The cover letter" in GitGitGadget
section
MyFirstContribution: drop PR description for GGG single-patch
contributions
Quoted 6 lines
I just realized that the additional/tweaked content that I added in v3 went into 2/5 where as they should have gone into 1/5 (I'm talking about the changes *before* the "The cover letter" heading in [1]). I slightly messed up my rebase it seems, I'm sorry...
Should I send a new version, or is it too late since it's now in 'next' ?
Thanks for letting me know, and sorry about the confusion.
Could you please send an incremental fix to be queued on top of these 5 patches?
Thanks.
Hi Junio,
Le 2022-05-21 à 21:35, Junio C Hamano a écrit :
Quoted 15 lines
Quoted 13 lines · Philippe Blain
Quoted 8 lines
Quoted 8 lines
Philippe Blain (5):
MyFirstContribution: add "Anatomy of a Patch Series" section
MyFirstContribution: add standalone section on cover letter
MyFirstContribution: reference "The cover letter" in "Preparing Email"
MyFirstContribution: reference "The cover letter" in GitGitGadget
section
MyFirstContribution: drop PR description for GGG single-patch
contributionsI just realized that the additional/tweaked content that I added in v3 went into 2/5 where as they should have gone into 1/5 (I'm talking about the changes *before* the "The cover letter" heading in [1]). I slightly messed up my rebase it seems, I'm sorry...
Quoted 6 lines
Quoted 1 line
Should I send a new version, or is it too late since it's now in 'next' ?
Thanks for letting me know, and sorry about the confusion.
Could you please send an incremental fix to be queued on top of these 5 patches?
I can't really send an incremental fix because the final content as it is in 'next' already is OK; it's just that some content which was added in patch 2/5 should have been added in 1/5 instead.
It's that too big of a deal, I just wanted to let you know.
Thanks,
Philippe.
Quoted 5 lines · Philippe Blain
I can't really send an incremental fix because the final content as it is in 'next' already is OK; it's just that some content which was added in patch 2/5 should have been added in 1/5 instead.
It's that too big of a deal, I just wanted to let you know.
Ah, I mis-read you then.
Such a misdivision may technically break bisectability, but hopefully nobody bisects the documentation changes and I do not think we should be worried too much about it.
Thanks.