diff mbox series

[bug#58648,v2] doc: contributing: Expand "Sending a Patch Series".

Message ID 20221020141349.4780-1-paren@disroot.org
State New
Headers show
Series [bug#58648,v2] doc: contributing: Expand "Sending a Patch Series". | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git-branch success View Git branch
cbaines/applying patch success
cbaines/issue success View issue
cbaines/comparison success View comparision
cbaines/git-branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

\( Oct. 20, 2022, 2:13 p.m. UTC
* doc/contributing.texi: Expand on sending patches and using
  git send-email.
---
 doc/contributing.texi | 120 ++++++++++++++++++++++++++++++------------
 1 file changed, 86 insertions(+), 34 deletions(-)


base-commit: 88746cd80bc56212ae7922c0fa1cd9a18e44c3bb

Comments

Liliana Marie Prikler Oct. 22, 2022, 11:21 a.m. UTC | #1
Am Donnerstag, dem 20.10.2022 um 15:13 +0100 schrieb (:
> * doc/contributing.texi: Expand on sending patches and using
>   git send-email.
> ---
>  doc/contributing.texi | 120 ++++++++++++++++++++++++++++++----------
> --
>  1 file changed, 86 insertions(+), 34 deletions(-)
> 
> diff --git a/doc/contributing.texi b/doc/contributing.texi
> index 4b1eed1cb1..650d3430fb 100644
> --- a/doc/contributing.texi
> +++ b/doc/contributing.texi
> @@ -1149,15 +1149,6 @@ Before submitting a patch that adds or
> modifies a package definition,
>  please run through this check list:
>  
>  @enumerate
> -@cindex @code{git format-patch}
> -@cindex @code{git-format-patch}
> -@item
> -When generating your patches with @code{git format-patch} or
> @code{git
> -send-email}, we recommend using the option @code{--base=}, perhaps
> with
> -the value @code{auto}.  This option adds a note to the patch stating
> -which commit the patch is based on.  This helps reviewers understand
> how
> -to apply and review your patches.
> -
>  @item
>  If the authors of the packaged software provide a cryptographic
>  signature for the release tarball, make an effort to verify the
> @@ -1343,18 +1334,6 @@ a subject, if your patch is to be applied on a
> branch other than
>  @code{master}, say @code{core-updates}, specify it in the subject
> like
>  @samp{[PATCH core-updates] @dots{}}.
>  
> -@quotation Tip
> -To add a prefix to the subject of your patch, you may use the
> -@option{--subject-prefix} option of the @command{git format-patch}
> or
> -@command{git send-email} commands, for example:
> -@example
> -git send-email --subject-prefix='PATCH core-updates' \
> -  --to=guix-patches@@gnu.org -1
> -@end example
> -For more information, run @samp{man git-format-patch} and @samp{man
> -git-send-email}.
> -@end quotation
> -
>  You may use your email client or the @command{git send-email}
> command
>  (@pxref{Sending a Patch Series}).  We prefer to get patches in plain
>  text messages, either inline or as MIME attachments.  You are
> advised to
> @@ -1409,19 +1388,92 @@ git config --local sendemail.thread no
>  @anchor{Sending a Patch Series}
>  @cindex patch series
>  @cindex @code{git send-email}
> +The @command{git send-email} and @command{git format-patch} commands
> allow
> +you to send your commits in email form to a mailing list, to be
> reviewed
> +and applied, and they are the recommended way to submit
> contributions to
> +Guix. When you send the first revision of a patch series, it's best
> to use
> +@command{git format-patch --cover-letter}.
> +
> +@example
> +$ git format-patch -$N -o outgoing --cover-letter --base=HEAD~$N
> +@end example
> +
> +@quotation Note
> +The @command{git send-email} command is provided by the @code{send-
> email}
> +output of the @code{git} package, i.e. @code{git:send-email}.
> +@end quotation
> +
> +This command makes patches out of the last @var{N} commits, and
> writes
> +them to @file{*.patch} files in @file{outgoing/}, along with an
> +automatically generated cover letter.  The @option{--base=HEAD~$N}
> option
> +adds @code{base-commit: @var{COMMIT}} to the bottom of the first
> email.
> +
> +We can now send the cover letter to the Guix mailing list, using
> +@command{git send-email}.
> +
> +@example
> +$ git send-email outgoing/0000-cover-letter.patch -a --to guix-
> patches@@gnu.org
> +@end example
> +
> +Note the @option{-a} flag; this pops up your editor so that you can
> fill
> +in the patchset subject line and blurb with whatever explanatory
> text you
> +feel is appropriate.  Note the automatically generated shortlog and
> +diffstat below the blurb, which help to give potential reviewers an
> +overview of the patchset.
> +
> +@quotation Tip
> +To add a prefix to the subject of your patch, you may use the
> +@option{--subject-prefix} option.
> +
> +@example
> +git format-patch -$N -o outgoing \
> +    --subject-prefix='PATCH core-updates' \
> +    --base=auto --cover-letter
> +@end example
> +@end quotation
> +
> +The use of the @file{etc/teams.scm} script to notify the appropriate
> team
> +members (@pxref{Teams}) is recommended when submitting patches, to
> maximize
> +the chances of your patch series being reviewed quickly.
I think you should also elaborate how this script is to be used.

> +At some point, the Debbugs mailer will reply to your cover letter
> mail
> +with an acknowledgement, which contains the issue number of your
> patchset.
> +You should now send the rest of the patches to this issue thread,
> using
> +the @email{@var{ISSUE_NUMBER}@@debbugs.gnu.org} address.
> +
> +@example
> +$ rm outgoing/0000-cover-letter.patch # Don't resend the cover
> letter!
> +$ git send-email outgoing/*.patch --to ISSUE_NUMBER@@debbugs.gnu.org
> +$ rm -rf outgoing # We're done with the patch files now.
> +@end example
> +
> +After a moment, your patches should appear at
> +@url{https://issues.guix.gnu.org/@var{ISSUE_NUMBER}} and
> +@url{https://debbugs.gnu.org/@var{ISSUE_NUMBER}}.
> +
> +@quotation Note
> +You should @strong{never} send all your patches to
> +@email{guix-patches@@gnu.org} at once, as this will create an issue
> for
> +each individual patch you send!  If you do accidentally do this,
> though,
> +it's not a massive problem, as Debbugs supports merging issues.
> +@end quotation
Perhaps 
  @quotation Caution
  Do @strong{not} send all your patches at once, as this will create an
  issue for each individual patch.  While Debbugs supports merging 
  issues, some interfaces (e.g. Mumi) still treat them as separate
  in certain ways, which can lower or exaggerate visibility and annoy
  reviewers.
  @end quotation

> +To incorporate a reviewer's suggestions, use @command{git rebase -i}
> to
> +amendthe commits, as demonstrated @url{https://git-rebase.io, here},
> and
amend the
> +send a second patch series with a @option{-v2} tag.
> +
> +@example
> +$ git send-email -$N -v2 --base=auto --to
> ISSUE_NUMBER@@debbugs.gnu.org
> +@end example
>  
> -When sending a patch series (e.g., using @code{git send-email}),
> please
> -first send one message to @email{guix-patches@@gnu.org}, and then
> send
> -subsequent patches to @email{@var{NNN}@@debbugs.gnu.org} to make
> sure
> -they are kept together.  See
> -@uref{https://debbugs.gnu.org/Advanced.html, the Debbugs
> documentation}
> -for more information.  You can install @command{git send-email} with
> -@command{guix install git:send-email}.
> -@c Debbugs bug: https://debbugs.gnu.org/db/15/15361.html
> +Note that since we already have an issue on Debbugs for our
> patchset,
> +there's no need for the intermediate @command{git format-patch}
> step. Of
> +course, to send a third patchset, you amy use @option{-v3}, to send
> a
> +fourth, @option{-v4}, and so on.
>  
> -To maximize the chances that you patch series is reviewed, the
> preferred
> -submission way is to use the @code{etc/teams.scm} script to notify
> the
> -appropriate team members (@pxref{Teams}).
> +If need be, you may use @option{--cover-letter -a} to send another
> cover
> +letter, e.g. for explaining what's changed since the last revision,
> and
> +these changes are necessary.
>  
>  @unnumberedsubsec Teams
>  @anchor{Teams}
> @@ -1448,7 +1500,7 @@ You can run the following command to have the
> @code{Mentors} team put in
>  CC of a patch series:
>  
>  @example
> -$ git send-email --to XXX@@debbugs.gnu.org $(./etc/teams.scm cc
> mentors) *.patch
> +$ git send-email --to @var{ISSUE_NUMBER}@@debbugs.gnu.org
> $(./etc/teams.scm cc mentors) *.patch
>  @end example
>  
>  The appropriate team or teams can also be inferred from the modified
> @@ -1457,7 +1509,7 @@ current Git repository to review, you can run:
>  
>  @example
>  $ guix shell -D guix
> -[env]$ git send-email --to XXX@@debbugs.gnu.org $(./etc/teams.scm
> cc-members HEAD~2 HEAD) *.patch
> +[env]$ git send-email --to @var{ISSUE_NUMBER}@@debbugs.gnu.org
> $(./etc/teams.scm cc-members HEAD~2 HEAD) *.patch
>  @end example
>  
>  @node Tracking Bugs and Patches
> 
> base-commit: 88746cd80bc56212ae7922c0fa1cd9a18e44c3bb
Also, this series makes it look as though we're only considering multi-
patch series.  Is the simple case of a single patch still covered?  I'd
perhaps organize this along these lines:
- Sending a single patch
- Notifying your senpais via etc/teams
- Sending a multi-patch series
Please don't take these headings too literally ;)

Cheers
\( Oct. 22, 2022, 11:30 a.m. UTC | #2
On Sat Oct 22, 2022 at 12:21 PM BST, Liliana Marie Prikler wrote:
> Also, this series makes it look as though we're only considering multi-
> patch series.  Is the simple case of a single patch still covered?

$ git format-patch -1 --cover-letter --base=auto -o outgoing

works fine for a single patch.  In my opinion, you should send a cover
letter for every set of patches, even if the set only contains a single
patch.

    -- (
Liliana Marie Prikler Oct. 22, 2022, 12:20 p.m. UTC | #3
Am Samstag, dem 22.10.2022 um 12:30 +0100 schrieb (:
> On Sat Oct 22, 2022 at 12:21 PM BST, Liliana Marie Prikler wrote:
> > Also, this series makes it look as though we're only considering
> > multi-
> > patch series.  Is the simple case of a single patch still covered?
> 
> $ git format-patch -1 --cover-letter --base=auto -o outgoing
> 
> works fine for a single patch.  In my opinion, you should send a
> cover letter for every set of patches, even if the set only contains
> a single patch.
> 
>     -- (
I kindly disagree.  With single patches, you can add notes below the
dashed line and send less bytes over avian carriers.
Simon Tournier Oct. 28, 2022, 2:41 p.m. UTC | #4
Hi Liliana,

On sam., 22 oct. 2022 at 14:20, Liliana Marie Prikler <liliana.prikler@gmail.com> wrote:

>> works fine for a single patch.  In my opinion, you should send a
>> cover letter for every set of patches, even if the set only contains
>> a single patch.

> I kindly disagree.  With single patches, you can add notes below the
> dashed line and send less bytes over avian carriers.

Well, the section is a recommendation for helping in contributions.
Therefore, I also think it is a good idea to recommend to start by a
cover letter even for single patch.  It does not mean that some other
workflows are not possible. :-)

Cheers,
simon
diff mbox series

Patch

diff --git a/doc/contributing.texi b/doc/contributing.texi
index 4b1eed1cb1..650d3430fb 100644
--- a/doc/contributing.texi
+++ b/doc/contributing.texi
@@ -1149,15 +1149,6 @@  Before submitting a patch that adds or modifies a package definition,
 please run through this check list:
 
 @enumerate
-@cindex @code{git format-patch}
-@cindex @code{git-format-patch}
-@item
-When generating your patches with @code{git format-patch} or @code{git
-send-email}, we recommend using the option @code{--base=}, perhaps with
-the value @code{auto}.  This option adds a note to the patch stating
-which commit the patch is based on.  This helps reviewers understand how
-to apply and review your patches.
-
 @item
 If the authors of the packaged software provide a cryptographic
 signature for the release tarball, make an effort to verify the
@@ -1343,18 +1334,6 @@  a subject, if your patch is to be applied on a branch other than
 @code{master}, say @code{core-updates}, specify it in the subject like
 @samp{[PATCH core-updates] @dots{}}.
 
-@quotation Tip
-To add a prefix to the subject of your patch, you may use the
-@option{--subject-prefix} option of the @command{git format-patch} or
-@command{git send-email} commands, for example:
-@example
-git send-email --subject-prefix='PATCH core-updates' \
-  --to=guix-patches@@gnu.org -1
-@end example
-For more information, run @samp{man git-format-patch} and @samp{man
-git-send-email}.
-@end quotation
-
 You may use your email client or the @command{git send-email} command
 (@pxref{Sending a Patch Series}).  We prefer to get patches in plain
 text messages, either inline or as MIME attachments.  You are advised to
@@ -1409,19 +1388,92 @@  git config --local sendemail.thread no
 @anchor{Sending a Patch Series}
 @cindex patch series
 @cindex @code{git send-email}
+The @command{git send-email} and @command{git format-patch} commands allow
+you to send your commits in email form to a mailing list, to be reviewed
+and applied, and they are the recommended way to submit contributions to
+Guix. When you send the first revision of a patch series, it's best to use
+@command{git format-patch --cover-letter}.
+
+@example
+$ git format-patch -$N -o outgoing --cover-letter --base=HEAD~$N
+@end example
+
+@quotation Note
+The @command{git send-email} command is provided by the @code{send-email}
+output of the @code{git} package, i.e. @code{git:send-email}.
+@end quotation
+
+This command makes patches out of the last @var{N} commits, and writes
+them to @file{*.patch} files in @file{outgoing/}, along with an
+automatically generated cover letter.  The @option{--base=HEAD~$N} option
+adds @code{base-commit: @var{COMMIT}} to the bottom of the first email.
+
+We can now send the cover letter to the Guix mailing list, using
+@command{git send-email}.
+
+@example
+$ git send-email outgoing/0000-cover-letter.patch -a --to guix-patches@@gnu.org
+@end example
+
+Note the @option{-a} flag; this pops up your editor so that you can fill
+in the patchset subject line and blurb with whatever explanatory text you
+feel is appropriate.  Note the automatically generated shortlog and
+diffstat below the blurb, which help to give potential reviewers an
+overview of the patchset.
+
+@quotation Tip
+To add a prefix to the subject of your patch, you may use the
+@option{--subject-prefix} option.
+
+@example
+git format-patch -$N -o outgoing \
+    --subject-prefix='PATCH core-updates' \
+    --base=auto --cover-letter
+@end example
+@end quotation
+
+The use of the @file{etc/teams.scm} script to notify the appropriate team
+members (@pxref{Teams}) is recommended when submitting patches, to maximize
+the chances of your patch series being reviewed quickly.
+
+At some point, the Debbugs mailer will reply to your cover letter mail
+with an acknowledgement, which contains the issue number of your patchset.
+You should now send the rest of the patches to this issue thread, using
+the @email{@var{ISSUE_NUMBER}@@debbugs.gnu.org} address.
+
+@example
+$ rm outgoing/0000-cover-letter.patch # Don't resend the cover letter!
+$ git send-email outgoing/*.patch --to ISSUE_NUMBER@@debbugs.gnu.org
+$ rm -rf outgoing # We're done with the patch files now.
+@end example
+
+After a moment, your patches should appear at
+@url{https://issues.guix.gnu.org/@var{ISSUE_NUMBER}} and
+@url{https://debbugs.gnu.org/@var{ISSUE_NUMBER}}.
+
+@quotation Note
+You should @strong{never} send all your patches to
+@email{guix-patches@@gnu.org} at once, as this will create an issue for
+each individual patch you send!  If you do accidentally do this, though,
+it's not a massive problem, as Debbugs supports merging issues.
+@end quotation
+
+To incorporate a reviewer's suggestions, use @command{git rebase -i} to
+amendthe commits, as demonstrated @url{https://git-rebase.io, here}, and
+send a second patch series with a @option{-v2} tag.
+
+@example
+$ git send-email -$N -v2 --base=auto --to ISSUE_NUMBER@@debbugs.gnu.org
+@end example
 
-When sending a patch series (e.g., using @code{git send-email}), please
-first send one message to @email{guix-patches@@gnu.org}, and then send
-subsequent patches to @email{@var{NNN}@@debbugs.gnu.org} to make sure
-they are kept together.  See
-@uref{https://debbugs.gnu.org/Advanced.html, the Debbugs documentation}
-for more information.  You can install @command{git send-email} with
-@command{guix install git:send-email}.
-@c Debbugs bug: https://debbugs.gnu.org/db/15/15361.html
+Note that since we already have an issue on Debbugs for our patchset,
+there's no need for the intermediate @command{git format-patch} step. Of
+course, to send a third patchset, you amy use @option{-v3}, to send a
+fourth, @option{-v4}, and so on.
 
-To maximize the chances that you patch series is reviewed, the preferred
-submission way is to use the @code{etc/teams.scm} script to notify the
-appropriate team members (@pxref{Teams}).
+If need be, you may use @option{--cover-letter -a} to send another cover
+letter, e.g. for explaining what's changed since the last revision, and
+these changes are necessary.
 
 @unnumberedsubsec Teams
 @anchor{Teams}
@@ -1448,7 +1500,7 @@  You can run the following command to have the @code{Mentors} team put in
 CC of a patch series:
 
 @example
-$ git send-email --to XXX@@debbugs.gnu.org $(./etc/teams.scm cc mentors) *.patch
+$ git send-email --to @var{ISSUE_NUMBER}@@debbugs.gnu.org $(./etc/teams.scm cc mentors) *.patch
 @end example
 
 The appropriate team or teams can also be inferred from the modified
@@ -1457,7 +1509,7 @@  current Git repository to review, you can run:
 
 @example
 $ guix shell -D guix
-[env]$ git send-email --to XXX@@debbugs.gnu.org $(./etc/teams.scm cc-members HEAD~2 HEAD) *.patch
+[env]$ git send-email --to @var{ISSUE_NUMBER}@@debbugs.gnu.org $(./etc/teams.scm cc-members HEAD~2 HEAD) *.patch
 @end example
 
 @node Tracking Bugs and Patches