[v2,1/5] MyFirstContribution: add "Anatomy of a Patch Series" section

Commit Message

Philippe Blain May 11, 2022, 1:47 a.m. UTC

From: Philippe Blain <[e-mail]>

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.

Signed-off-by: Philippe Blain <[e-mail]>
---
 Documentation/MyFirstContribution.txt | 54 +++++++++++++++++++++++++--
 1 file changed, 51 insertions(+), 3 deletions(-)

Comments

Bagas Sanjaya May 11, 2022, 6:20 a.m. UTC | #1

On Wed, May 11, 2022 at 01:47:30AM +0000, Philippe Blain via GitGitGadget wrote:
> +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.
>

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.

Junio C Hamano May 11, 2022, 9:30 p.m. UTC | #2

Bagas Sanjaya <[e-mail]> writes:

> On Wed, May 11, 2022 at 01:47:30AM +0000, Philippe Blain via GitGitGadget wrote:
>> +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.
>>
>
> 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.

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

Philippe Blain May 11, 2022, 9:48 p.m. UTC | #3

Hi Bagas and Junio,

Le 2022-05-11 à 17:30, Junio C Hamano a écrit :
> Bagas Sanjaya <[e-mail]> writes:
> 
>> On Wed, May 11, 2022 at 01:47:30AM +0000, Philippe Blain via GitGitGadget wrote:
>>> +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.
>>>
>>
>> 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.

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

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

Junio C Hamano May 11, 2022, 10:09 p.m. UTC | #4

Philippe Blain <[e-mail]> writes:

>> Bagas Sanjaya <[e-mail]> writes:
>> 
>>> On Wed, May 11, 2022 at 01:47:30AM +0000, Philippe Blain via GitGitGadget wrote:
>>>> +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.
>>>>
>>>
>>> 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.
>
> 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

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

Philip Oakley May 12, 2022, 12:11 p.m. UTC | #5

On 11/05/2022 02:47, Philippe Blain via GitGitGadget wrote:
> diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
> index 63a2ef54493..22848f84bec 100644
> --- a/Documentation/MyFirstContribution.txt
> +++ b/Documentation/MyFirstContribution.txt
> @@ -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.

>   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
--
Philip

Philippe Blain May 12, 2022, 10:53 p.m. UTC | #6

Hi Philip,

Le 2022-05-12 à 08:11, Philip Oakley a écrit :
> On 11/05/2022 02:47, Philippe Blain via GitGitGadget wrote:
>> diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
>> index 63a2ef54493..22848f84bec 100644
>> --- a/Documentation/MyFirstContribution.txt
>> +++ b/Documentation/MyFirstContribution.txt
>> @@ -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.

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.

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

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.

Philippe Blain May 12, 2022, 11 p.m. UTC | #7

Hi Junio,

Le 2022-05-11 à 18:09, Junio C Hamano a écrit :
> Philippe Blain <[e-mail]> writes:
> 
>>> Bagas Sanjaya <[e-mail]> writes:
>>>
>>>> On Wed, May 11, 2022 at 01:47:30AM +0000, Philippe Blain via GitGitGadget wrote:
>>>>> +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.
>>>>>
>>>>
>>>> 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.
>>
>> 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.

OK, a little side note sounds good.

> That reminds me of one small bug in your patch, where it says
> 
>>>>> +- 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 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.

Patch

diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
index 63a2ef54493..22848f84bec 100644
--- a/Documentation/MyFirstContribution.txt
+++ b/Documentation/MyFirstContribution.txt
@@ -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/[e-mail]/
+: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]:
+
+----
[phone]:40 [PATCH 0/3] libify reflog John Cai via GitGitGadget
[phone]:40 ` [PATCH 1/3] reflog: libify delete reflog function and helpers John Cai via GitGitGadget
[phone]:10   ` Ævar Arnfjörð Bjarmason [this message]
[phone]:39     ` Taylor Blau
[phone]:48       ` Ævar Arnfjörð Bjarmason
[phone]:35   ` Taylor Blau
[phone]:43     ` John Cai
[phone]:50       ` Taylor Blau
[phone]:50         ` John Cai
[phone]:00   ` // other replies ellided
[phone]:40 ` [PATCH 2/3] reflog: call reflog_delete from reflog.c John Cai via GitGitGadget
[phone]:15   ` Ævar Arnfjörð Bjarmason
[phone]:26     ` Junio C Hamano
[phone]:40 ` [PATCH 3/3] stash: call reflog_delete from reflog.c John Cai via GitGitGadget
[phone]:20   ` Ævar Arnfjörð Bjarmason
[phone]:21     ` Taylor Blau
[phone]:36     ` John Cai
[phone]:51       ` Ævar Arnfjörð Bjarmason
[phone]:29 ` [PATCH 0/3] libify reflog Ævar Arnfjörð Bjarmason
[phone]:30 ` [PATCH v2 0/3] libify reflog John Cai via GitGitGadget
[phone]:30   ` [PATCH v2 1/3] stash: add test to ensure reflog --rewrite --updatref behavior John Cai via GitGitGadget
[phone]:54     ` Ævar Arnfjörð Bjarmason
[phone]: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