[libvirt] [PATCHv2] docs: Improve patch submission guidelines

[Eric Blake]

On 07/10/2012 08:32 AM, Michal Privoznik wrote:
> We should really advise (new) developers to send rebased patches
> that apply cleanly and use git-send-email rather than all other
> obscure ways.
> ---
>  HACKING              |   22 +++++++++++++++++++++-
>  docs/hacking.html.in |   33 ++++++++++++++++++++++++++++-----
>  2 files changed, 49 insertions(+), 6 deletions(-)

I kind of reviewed the generated output, although obviously any changes
would be to the original source html.in file.

> 
> diff --git a/HACKING b/HACKING
> index 69ea96b..def94ee 100644
> --- a/HACKING
> +++ b/HACKING
> @@ -21,9 +21,29 @@ or:
>  
>    git diff > libvirt-myfeature.patch
>  
> +However, the usual workflow of libvirt developer is:  git checkout master

Hmm, I would have expected a new line here.  Wonder what it is about our
xslt conversion that merged these, and if there is anything we can do in
the .in file to force the line break?

> +  git pull
> +  git checkout -b workbranch
> +  Hack, committing any changes along the way
> +
> +Then, when you want to post your patches:  git checkout master
> +  git pull
> +  git checkout workbranch
> +  git rebase master

These can be shortened; if you do:

git checkout -t origin -b workbranch

to originally create your workbranch, then preparing to post is as
simple as:

git pull --rebase

rather than jumping through two more checkouts.

> +  (fix any conflicts)
> +  git send-email --compose --to=libvir-list at redhat.com master

I actually like 'send-email --cover-letter --annotate' better than
'send-email --compose'.  I also recommend doing:

git config sendemail.to libvir-list at redhat.com

so that you don't have to type the --to=... designation every time.

Maybe also mention that a single patch doesn't need a cover letter, but
a series of 2 or more does.

> +
> +Please follow this as close as you can, especially the rebase and git
> +send-email part as it makes developer's, who is reviewing your patch set, life

Awkward read.  How about:

especially the rebase and git send-email part, as it makes life easier
for other developers to review your patch set.

> +easier. One should avoid sending patches as attachment but rather send them in

s/attachment/attachments,/

> +email body among with commit message.

Mention that patch versioning information (such as a resend of a series
to address review comments from the first version) should occur after a
'---' line in the email, so that it helps reviewers but doesn't become
part of git history.  Also mention that 'git send-email
--subject-prefix=PATCHv2' can be useful for annotating revised patches.

> +
>  (3) Split large changes into a series of smaller patches, self-contained if
>  possible, with an explanation of each patch and an explanation of how the
> -sequence of patches fits together.
> +sequence of patches fits together. Moreover, please keep in mind that it's
> +required to be able to compile cleanly after each patch.

good, but I'd also add:

A feature does not have to work until the end of a series, as long as
intermediate patches don't cause test-suite failures.


> +  Hack, committing any changes along the way
> +</pre>
> +        Then, when you want to post your patches:
> +<pre>

Per my above comments, this may need some <p> markup.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org



-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120710/9a3beb88/attachment-0001.sig>