[libvirt] [PATCHv2] docs: Improve patch submission guidelines
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(-) 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 + 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 + (fix any conflicts) + git send-email --compose --to=libvir-list@redhat.com master + +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 +easier. One should avoid sending patches as attachment but rather send them in +email body among with commit message. + (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. + + (4) Make sure your patches apply against libvirt GIT. Developers only follow GIT and don't care much about released versions. diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 89f9980..af852e0 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -11,19 +11,42 @@ <li><p>Post patches in unified diff format. A command similar to this should work:</p> -<pre> +<del><pre> diff -urp libvirt.orig/ libvirt.modified/ > libvirt-myfeature.patch -</pre> +</pre></del> <p> or: </p> <pre> git diff > libvirt-myfeature.patch </pre> - </li> - <li>Split large changes into a series of smaller patches, self-contained + However, the usual workflow of libvirt developer is: +<pre> + git checkout master + git pull + git checkout -b workbranch + Hack, committing any changes along the way +</pre> + Then, when you want to post your patches: +<pre> + git checkout master + git pull + git checkout workbranch + git rebase master + (fix any conflicts) + git send-email --compose --no-chain-reply-to --to=libvir-list@redhat.com master +</pre> + <p>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 easier. One should avoid sending patches as attachment but + rather send them in email body among with commit message.</p></li> + + <li><p>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.</li> + the sequence of patches fits together. Moreover, please keep in mind that + it's required to be able to compile cleanly after each patch.</p> + </li> + <li>Make sure your patches apply against libvirt GIT. Developers only follow GIT and don't care much about released versions.</li> <li><p>Run the automated tests on your code before submitting any changes. -- 1.7.8.6
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@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@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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Michal Privoznik