[libvirt] [PATCH] hacking: Improve 'git send-email' documentation

Andrea Bolognani posted 1 patch 6 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1497497503-10453-1-git-send-email-abologna@redhat.com
There is a newer version of this series
docs/hacking.html.in | 36 ++++++++++++++++++++++++------------
1 file changed, 24 insertions(+), 12 deletions(-)
[libvirt] [PATCH] hacking: Improve 'git send-email' documentation
Posted by Andrea Bolognani 6 years, 9 months ago
For the benefit of first time contributors, we point out that 'git
send-email' might have to be installed separately; however, we omit
the fact that some configuration will likely be needed before it
can successfully deliver patches to the mailing list.

Some minor tweaks to the existing contents are included as well.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 docs/hacking.html.in | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index d6a574c..434fb68 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -29,8 +29,8 @@
         file from zanata.</p>
       </li>
 
-      <li><p>Post patches using "git send-email", with git rename
-        detection enabled.  You need a one-time setup of:</p>
+      <li><p>Post patches using <code>git send-email</code>, with git
+        rename detection enabled.  You need a one-time setup of:</p>
 <pre>
   git config diff.renames true
 </pre>
@@ -52,20 +52,32 @@
   git send-email --cover-letter --no-chain-reply-to --annotate \
                  --to=libvir-list@redhat.com master
 </pre>
-        <p>(Note that the "git send-email" subcommand may not be in
-        the main git package and using it may require installation of a
-        separate package, for example the "git-email" package in
-        Fedora.)  For a single patch you can omit
-        <code>--cover-letter</code>, but a series of two or more
-        patches needs a cover letter. If you get tired of typing
-        <code>--to=libvir-list@redhat.com</code> designation you can
-        set it in git config:</p>
+        <p>Note that the <code>git send-email</code> subcommand may not
+        be in the main git package and using it may require installation
+        of a separate package, for example the "git-email" package in
+        Fedora and Debian.  If this is your first time using
+        <code>git send-email</code>, you might need to configure it to
+        point it to your SMTP server with something like:</p>
+<pre>
+  git config --global sendemail.smtpServer stmp.youremailprovider.net
+</pre>
+        <p>If you get tired of typing
+        <code>--to=libvir-list@redhat.com</code> all the time, you can
+        configure that to be automatically handled as well:</p>
 <pre>
   git config sendemail.to libvir-list@redhat.com
 </pre>
+        <p>For a single patch you can omit
+        <code>--cover-letter</code>, but a series of two or more
+        patches needs a cover letter.</p>
+        <p>If everything went well, your patch should show up on the
+        <a href="https://www.redhat.com/archives/libvir-list/">libvir-list
+        archives</a> in a matter of minutes; if you still can't find it on
+        there after an hour or so, you should double-check your setup.</p>
         <p>Please follow this as close as you can, especially the rebase and
-        git send-email part, as it makes life easier for other developers to
-        review your patch set. One should avoid sending patches as attachments,
+        <code>git send-email</code> part, as it makes life easier for other
+        developers to review your patch set.
+        One should avoid sending patches as attachments,
         but rather send them in email body along with commit message. If a
         developer is sending another version of the patch (e.g. to address
         review comments), they are advised to note differences to previous
-- 
2.7.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] hacking: Improve 'git send-email' documentation
Posted by Ján Tomko 6 years, 9 months ago
On Thu, Jun 15, 2017 at 11:31:43AM +0800, Andrea Bolognani wrote:
>For the benefit of first time contributors, we point out that 'git
>send-email' might have to be installed separately; however, we omit
>the fact that some configuration will likely be needed before it
>can successfully deliver patches to the mailing list.
>
>Some minor tweaks to the existing contents are included as well.
>
>Signed-off-by: Andrea Bolognani <abologna@redhat.com>
>---
> docs/hacking.html.in | 36 ++++++++++++++++++++++++------------
> 1 file changed, 24 insertions(+), 12 deletions(-)
>
> <pre>
>   git config sendemail.to libvir-list@redhat.com
> </pre>
>+        <p>For a single patch you can omit
>+        <code>--cover-letter</code>, but a series of two or more
>+        patches needs a cover letter.</p>
>+        <p>If everything went well, your patch should show up on the
>+        <a href="https://www.redhat.com/archives/libvir-list/">libvir-list
>+        archives</a> in a matter of minutes; if you still can't find it on
>+        there after an hour or so, you should double-check your setup.</p>

First time posts to the mailing list still have to be approved by the
moderator, AFAIK.

Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] hacking: Improve 'git send-email' documentation
Posted by Kashyap Chamarthy 6 years, 9 months ago
On Thu, Jun 15, 2017 at 11:31:43AM +0800, Andrea Bolognani wrote:
> For the benefit of first time contributors, we point out that 'git
> send-email' might have to be installed separately; however, we omit
> the fact that some configuration will likely be needed before it
> can successfully deliver patches to the mailing list.
> 
> Some minor tweaks to the existing contents are included as well.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  docs/hacking.html.in | 36 ++++++++++++++++++++++++------------
>  1 file changed, 24 insertions(+), 12 deletions(-)

[...]

> -        <p>(Note that the "git send-email" subcommand may not be in
> -        the main git package and using it may require installation of a
> -        separate package, for example the "git-email" package in
> -        Fedora.)  For a single patch you can omit
> -        <code>--cover-letter</code>, but a series of two or more
> -        patches needs a cover letter. 

For multiple patch series, is it worthwhile to mention the "--stat"
option to include the diffstat in the cover letter?  When reading a
patch series, I find it useful to see the diffstat at a glance.

E.g. when sending the top 4 commits:

    $ git send-email -4 --cover-letter --no-chain-reply-to \
        --annotate --to=libvir@redhat.com master --stat

Maybe others here have their own preferred method to include diffstat in
cover letters.

[...]

-- 
/kashyap

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] hacking: Improve 'git send-email' documentation
Posted by Andrea Bolognani 6 years, 9 months ago
On Thu, 2017-06-15 at 11:08 +0200, Kashyap Chamarthy wrote:
> For multiple patch series, is it worthwhile to mention the "--stat"
> option to include the diffstat in the cover letter?  When reading a
> patch series, I find it useful to see the diffstat at a glance.
> 
> E.g. when sending the top 4 commits:
> 
>     $ git send-email -4 --cover-letter --no-chain-reply-to \
>         --annotate --to=libvir@redhat.com master --stat
> 
> Maybe others here have their own preferred method to include diffstat in
> cover letters.

The diffstat seems to be generated automatically for me.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] hacking: Improve 'git send-email' documentation
Posted by John Ferlan 6 years, 9 months ago

On 06/14/2017 11:31 PM, Andrea Bolognani wrote:
> For the benefit of first time contributors, we point out that 'git
> send-email' might have to be installed separately; however, we omit
> the fact that some configuration will likely be needed before it
> can successfully deliver patches to the mailing list.
> 
> Some minor tweaks to the existing contents are included as well.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  docs/hacking.html.in | 36 ++++++++++++++++++++++++------------
>  1 file changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/docs/hacking.html.in b/docs/hacking.html.in
> index d6a574c..434fb68 100644
> --- a/docs/hacking.html.in
> +++ b/docs/hacking.html.in
> @@ -29,8 +29,8 @@
>          file from zanata.</p>
>        </li>
>  
> -      <li><p>Post patches using "git send-email", with git rename
> -        detection enabled.  You need a one-time setup of:</p>
> +      <li><p>Post patches using <code>git send-email</code>, with git
> +        rename detection enabled.  You need a one-time setup of:</p>
>  <pre>
>    git config diff.renames true
>  </pre>
> @@ -52,20 +52,32 @@
>    git send-email --cover-letter --no-chain-reply-to --annotate \
>                   --to=libvir-list@redhat.com master
>  </pre>

FWIW: I have in my cut-n-paste command "--confirm=always" - it just
makes sure I really want to send things, but has saved me a couple of
times!

> -        <p>(Note that the "git send-email" subcommand may not be in
> -        the main git package and using it may require installation of a
> -        separate package, for example the "git-email" package in
> -        Fedora.)  For a single patch you can omit
> -        <code>--cover-letter</code>, but a series of two or more
> -        patches needs a cover letter. If you get tired of typing
> -        <code>--to=libvir-list@redhat.com</code> designation you can
> -        set it in git config:</p>
> +        <p>Note that the <code>git send-email</code> subcommand may not
> +        be in the main git package and using it may require installation
> +        of a separate package, for example the "git-email" package in
> +        Fedora and Debian.  If this is your first time using
> +        <code>git send-email</code>, you might need to configure it to
> +        point it to your SMTP server with something like:</p>
> +<pre>
> +  git config --global sendemail.smtpServer stmp.youremailprovider.net
> +</pre>
> +        <p>If you get tired of typing
> +        <code>--to=libvir-list@redhat.com</code> all the time, you can
> +        configure that to be automatically handled as well:</p>
>  <pre>
>    git config sendemail.to libvir-list@redhat.com
>  </pre>

As long as we're adding stuff... For those that use libvir-list to post
patches from multiple repositories (e.g. libvirt-python, libvirt-perl,
etc.) using:

git config --local format.subjectprefix "libvirt-python PATCH"

(or libvirt-perl or whatever the repository is) really *helps* reviewers
focus whether the patch is meant for mainline libvirt or whether it's
for some other git repo.

Also (how strongly can we say the following)... Please, please, please,
pretty-please... Do *NOT* use the "--cc" option to copy random
developers. We're all subscribed to the list and will *definitely* see
the patches. I would even say I'd rather not see it for -vN patches too.

> +        <p>For a single patch you can omit
> +        <code>--cover-letter</code>, but a series of two or more
> +        patches needs a cover letter.</p>
> +        <p>If everything went well, your patch should show up on the
> +        <a href="https://www.redhat.com/archives/libvir-list/">libvir-list
> +        archives</a> in a matter of minutes; if you still can't find it on
> +        there after an hour or so, you should double-check your setup.</p>

Another "hint" I got when first starting out - send your patch to your
own email address instead of libvir-list in order to make sure it "looks
right" or "similar to" how patches are posted on the list.

One could also go as far as "saving" those patches and creating a new
git branch to test whether a "git am -3" would work on their patch(es).
That at least ensures someone else applying an on list patch would be
able to "use" whatever was posted.

Thanks for diving into the shallow end of pool and updating this ;-)

John

>          <p>Please follow this as close as you can, especially the rebase and
> -        git send-email part, as it makes life easier for other developers to
> -        review your patch set. One should avoid sending patches as attachments,
> +        <code>git send-email</code> part, as it makes life easier for other
> +        developers to review your patch set.
> +        One should avoid sending patches as attachments,
>          but rather send them in email body along with commit message. If a
>          developer is sending another version of the patch (e.g. to address
>          review comments), they are advised to note differences to previous
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list