2
0

submitting-a-patch.rst 28 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280281282283284285286287288289290291292293294295296297298299300301302303304305306307308309310311312313314315316317318319320321322323324325326327328329330331332333334335336337338339340341342343344345346347348349350351352353354355356357358359360361362363364365366367368369370371372373374375376377378379380381382383384385386387388389390391392393394395396397398399400401402403404405406407408409410411412413414415416417418419420421422423424425426427428429430431432433434435436437438439440441442443444445446447448449450451452453454455456457458459460461462463464465466467468469470471472473474475476477478479480481482483484485486487488489490491492493494495496497498499500501502503504505506507508509510511512513514515516517518519520521522523524525526527528529530531532533534535536537538539540541542543544545546547548549550551552553554555556557558559560561562563564565566567568569570571572573574575576577578579580581582583584585586587588589590591592593594595596597598599600601602603604605606607608609610611612613614615616617618619620621622623624625626627628629630631632633634
  1. .. _submitting-a-patch:
  2. Submitting a Patch
  3. ==================
  4. QEMU welcomes contributions to fix bugs, add functionality or improve
  5. the documentation. However, we get a lot of patches, and so we have
  6. some guidelines about submitting them. If you follow these, you'll
  7. help make our task of contribution review easier and your change is
  8. likely to be accepted and committed faster.
  9. This page seems very long, so if you are only trying to post a quick
  10. one-shot fix, the bare minimum we ask is that:
  11. .. list-table:: Minimal Checklist for Patches
  12. :widths: 35 65
  13. :header-rows: 1
  14. * - Check
  15. - Reason
  16. * - Patches contain Signed-off-by: Your Name <author@email>
  17. - States you are legally able to contribute the code. See :ref:`patch_emails_must_include_a_signed_off_by_line`
  18. * - Sent as patch emails to ``qemu-devel@nongnu.org``
  19. - The project uses an email list based workflow. See :ref:`submitting_your_patches`
  20. * - Be prepared to respond to review comments
  21. - Code that doesn't pass review will not get merged. See :ref:`participating_in_code_review`
  22. You do not have to subscribe to post (list policy is to reply-to-all to
  23. preserve CCs and keep non-subscribers in the loop on the threads they
  24. start), although you may find it easier as a subscriber to pick up good
  25. ideas from other posts. If you do subscribe, be prepared for a high
  26. volume of email, often over one thousand messages in a week. The list is
  27. moderated; first-time posts from an email address (whether or not you
  28. subscribed) may be subject to some delay while waiting for a moderator
  29. to allow your address.
  30. The larger your contribution is, or if you plan on becoming a long-term
  31. contributor, then the more important the rest of this page becomes.
  32. Reading the table of contents below should already give you an idea of
  33. the basic requirements. Use the table of contents as a reference, and
  34. read the parts that you have doubts about.
  35. .. contents:: Table of Contents
  36. .. _writing_your_patches:
  37. Writing your Patches
  38. --------------------
  39. .. _use_the_qemu_coding_style:
  40. Use the QEMU coding style
  41. ~~~~~~~~~~~~~~~~~~~~~~~~~
  42. You can run run *scripts/checkpatch.pl <patchfile>* before submitting to
  43. check that you are in compliance with our coding standards. Be aware
  44. that ``checkpatch.pl`` is not infallible, though, especially where C
  45. preprocessor macros are involved; use some common sense too. See also:
  46. - :ref:`coding-style`
  47. - `Automate a checkpatch run on
  48. commit <https://blog.vmsplice.net/2011/03/how-to-automatically-run-checkpatchpl.html>`__
  49. .. _base_patches_against_current_git_master:
  50. Base patches against current git master
  51. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  52. There's no point submitting a patch which is based on a released version
  53. of QEMU because development will have moved on from then and it probably
  54. won't even apply to master. We only apply selected bugfixes to release
  55. branches and then only as backports once the code has gone into master.
  56. It is also okay to base patches on top of other on-going work that is
  57. not yet part of the git master branch. To aid continuous integration
  58. tools, such as `patchew <http://patchew.org/QEMU/>`__, you should `add a
  59. tag <https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01288.html>`__
  60. line ``Based-on: $MESSAGE_ID`` to your cover letter to make the series
  61. dependency obvious.
  62. .. _split_up_long_patches:
  63. Split up long patches
  64. ~~~~~~~~~~~~~~~~~~~~~
  65. Split up longer patches into a patch series of logical code changes.
  66. Each change should compile and execute successfully. For instance, don't
  67. add a file to the makefile in patch one and then add the file itself in
  68. patch two. (This rule is here so that people can later use tools like
  69. `git bisect <http://git-scm.com/docs/git-bisect>`__ without hitting
  70. points in the commit history where QEMU doesn't work for reasons
  71. unrelated to the bug they're chasing.) Put documentation first, not
  72. last, so that someone reading the series can do a clean-room evaluation
  73. of the documentation, then validate that the code matched the
  74. documentation. A commit message that mentions "Also, ..." is often a
  75. good candidate for splitting into multiple patches. For more thoughts on
  76. properly splitting patches and writing good commit messages, see `this
  77. advice from
  78. OpenStack <https://wiki.openstack.org/wiki/GitCommitMessages>`__.
  79. .. _make_code_motion_patches_easy_to_review:
  80. Make code motion patches easy to review
  81. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  82. If a series requires large blocks of code motion, there are tricks for
  83. making the refactoring easier to review. Split up the series so that
  84. semantic changes (or even function renames) are done in a separate patch
  85. from the raw code motion. Use a one-time setup of ``git config
  86. diff.renames true;`` ``git config diff.algorithm patience`` (refer to
  87. `git-config <http://git-scm.com/docs/git-config>`__). The 'diff.renames'
  88. property ensures file rename patches will be given in a more compact
  89. representation that focuses only on the differences across the file
  90. rename, instead of showing the entire old file as a deletion and the new
  91. file as an insertion. Meanwhile, the 'diff.algorithm' property ensures
  92. that extracting a non-contiguous subset of one file into a new file, but
  93. where all extracted parts occur in the same order both before and after
  94. the patch, will reduce churn in trying to treat unrelated ``}`` lines in
  95. the original file as separating hunks of changes.
  96. Ideally, a code motion patch can be reviewed by doing::
  97. git format-patch --stdout -1 > patch;
  98. diff -u <(sed -n 's/^-//p' patch) <(sed -n 's/^\+//p' patch)
  99. to focus on the few changes that weren't wholesale code motion.
  100. .. _dont_include_irrelevant_changes:
  101. Don't include irrelevant changes
  102. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  103. In particular, don't include formatting, coding style or whitespace
  104. changes to bits of code that would otherwise not be touched by the
  105. patch. (It's OK to fix coding style issues in the immediate area (few
  106. lines) of the lines you're changing.) If you think a section of code
  107. really does need a reindent or other large-scale style fix, submit this
  108. as a separate patch which makes no semantic changes; don't put it in the
  109. same patch as your bug fix.
  110. For smaller patches in less frequently changed areas of QEMU, consider
  111. using the :ref:`trivial-patches` process.
  112. .. _write_a_meaningful_commit_message:
  113. Write a meaningful commit message
  114. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  115. Commit messages should be meaningful and should stand on their own as a
  116. historical record of why the changes you applied were necessary or
  117. useful.
  118. QEMU follows the usual standard for git commit messages: the first line
  119. (which becomes the email subject line) is "subsystem: single line
  120. summary of change". Whether the "single line summary of change" starts
  121. with a capital is a matter of taste, but we prefer that the summary does
  122. not end in a dot. Look at ``git shortlog -30`` for an idea of sample
  123. subject lines. Then there is a blank line and a more detailed
  124. description of the patch, another blank and your Signed-off-by: line.
  125. Please do not use lines that are longer than 76 characters in your
  126. commit message (so that the text still shows up nicely with "git show"
  127. in a 80-columns terminal window).
  128. The body of the commit message is a good place to document why your
  129. change is important. Don't include comments like "This is a suggestion
  130. for fixing this bug" (they can go below the ``---`` line in the email so
  131. they don't go into the final commit message). Make sure the body of the
  132. commit message can be read in isolation even if the reader's mailer
  133. displays the subject line some distance apart (that is, a body that
  134. starts with "... so that" as a continuation of the subject line is
  135. harder to follow).
  136. If your patch fixes a commit that is already in the repository, please
  137. add an additional line with "Fixes: <at-least-12-digits-of-SHA-commit-id>
  138. ("Fixed commit subject")" below the patch description / before your
  139. "Signed-off-by:" line in the commit message.
  140. If your patch fixes a bug in the gitlab bug tracker, please add a line
  141. with "Resolves: <URL-of-the-bug>" to the commit message, too. Gitlab can
  142. close bugs automatically once commits with the "Resolves:" keyword get
  143. merged into the master branch of the project. And if your patch addresses
  144. a bug in another public bug tracker, you can also use a line with
  145. "Buglink: <URL-of-the-bug>" for reference here, too.
  146. Example::
  147. Fixes: 14055ce53c2d ("s390x/tcg: avoid overflows in time2tod/tod2time")
  148. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/42
  149. Buglink: https://bugs.launchpad.net/qemu/+bug/1804323``
  150. Some other tags that are used in commit messages include "Message-Id:"
  151. "Tested-by:", "Acked-by:", "Reported-by:", "Suggested-by:". See ``git
  152. log`` for these keywords for example usage.
  153. .. _test_your_patches:
  154. Test your patches
  155. ~~~~~~~~~~~~~~~~~
  156. Although QEMU uses various :ref:`ci` services that attempt to test
  157. patches submitted to the list, it still saves everyone time if you
  158. have already tested that your patch compiles and works. Because QEMU
  159. is such a large project the default configuration won't create a
  160. testing pipeline on GitLab when a branch is pushed. See the :ref:`CI
  161. variable documentation<ci_var>` for details on how to control the
  162. running of tests; but it is still wise to also check that your patches
  163. work with a full build before submitting a series, especially if your
  164. changes might have an unintended effect on other areas of the code you
  165. don't normally experiment with. See :ref:`testing` for more details on
  166. what tests are available.
  167. Also, it is a wise idea to include a testsuite addition as part of
  168. your patches - either to ensure that future changes won't regress your
  169. new feature, or to add a test which exposes the bug that the rest of
  170. your series fixes. Keeping separate commits for the test and the fix
  171. allows reviewers to rebase the test to occur first to prove it catches
  172. the problem, then again to place it last in the series so that
  173. bisection doesn't land on a known-broken state.
  174. .. _submitting_your_patches:
  175. Submitting your Patches
  176. -----------------------
  177. The QEMU project uses a public email based workflow for reviewing and
  178. merging patches. As a result all contributions to QEMU must be **sent
  179. as patches** to the qemu-devel `mailing list
  180. <https://wiki.qemu.org/Contribute/MailingLists>`__. Patch
  181. contributions should not be posted on the bug tracker, posted on
  182. forums, or externally hosted and linked to. (We have other mailing
  183. lists too, but all patches must go to qemu-devel, possibly with a Cc:
  184. to another list.) ``git send-email`` (`step-by-step setup guide
  185. <https://git-send-email.io/>`__ and `hints and tips
  186. <https://elixir.bootlin.com/linux/latest/source/Documentation/process/email-clients.rst>`__)
  187. works best for delivering the patch without mangling it, but
  188. attachments can be used as a last resort on a first-time submission.
  189. .. _use_git_publish:
  190. Use git-publish
  191. ~~~~~~~~~~~~~~~
  192. If you already configured git send-email, you can simply use `git-publish
  193. <https://github.com/stefanha/git-publish>`__ to send series.
  194. ::
  195. $ git checkout master -b my-feature
  196. $ # work on new commits, add your 'Signed-off-by' lines to each
  197. $ git publish
  198. $ ... more work, rebase on master, ...
  199. $ git publish # will send a v2
  200. Each time you post a series, git-publish will create a local tag with the format
  201. ``<branchname>-v<version>`` to record the patch series.
  202. When sending patch emails, 'git publish' will consult the output of
  203. 'scripts/get_maintainers.pl' and automatically CC anyone listed as maintainers
  204. of the affected code. Generally you should accept the suggested CC list, but
  205. there may sometimes be scenarios where it is appropriate to cut it down (eg on
  206. certain large tree-wide cleanups), or augment it with other interested people.
  207. .. _if_you_cannot_send_patch_emails:
  208. If you cannot send patch emails
  209. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  210. In rare cases it may not be possible to send properly formatted patch
  211. emails. You can use `sourcehut <https://sourcehut.org/>`__ to send your
  212. patches to the QEMU mailing list by following these steps:
  213. #. Register or sign in to your account
  214. #. Add your SSH public key in `meta \|
  215. keys <https://meta.sr.ht/keys>`__.
  216. #. Publish your git branch using **git push git@git.sr.ht:~USERNAME/qemu
  217. HEAD**
  218. #. Send your patches to the QEMU mailing list using the web-based
  219. ``git-send-email`` UI at https://git.sr.ht/~USERNAME/qemu/send-email
  220. Documentation for sourcehut is available `here
  221. <https://man.sr.ht/git.sr.ht/#sending-patches-upstream>`__.
  222. .. _cc_the_relevant_maintainer:
  223. CC the relevant maintainer
  224. ~~~~~~~~~~~~~~~~~~~~~~~~~~
  225. Send patches both to the mailing list and CC the maintainer(s) of the
  226. files you are modifying. look in the MAINTAINERS file to find out who
  227. that is. Also try using scripts/get_maintainer.pl from the repository
  228. for learning the most common committers for the files you touched.
  229. Example::
  230. ~/src/qemu/scripts/get_maintainer.pl -f hw/ide/core.c
  231. In fact, you can automate this, via a one-time setup of ``git config
  232. sendemail.cccmd 'scripts/get_maintainer.pl --nogit-fallback'`` (Refer to
  233. `git-config <http://git-scm.com/docs/git-config>`__.)
  234. .. _do_not_send_as_an_attachment:
  235. Do not send as an attachment
  236. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  237. Send patches inline so they are easy to reply to with review comments.
  238. Do not put patches in attachments.
  239. .. _use_git_format_patch:
  240. Use ``git format-patch``
  241. ~~~~~~~~~~~~~~~~~~~~~~~~
  242. Use the right diff format.
  243. `git format-patch <http://git-scm.com/docs/git-format-patch>`__ will
  244. produce patch emails in the right format (check the documentation to
  245. find out how to drive it). You can then edit the cover letter before
  246. using ``git send-email`` to mail the files to the mailing list. (We
  247. recommend `git send-email <http://git-scm.com/docs/git-send-email>`__
  248. because mail clients often mangle patches by wrapping long lines or
  249. messing up whitespace. Some distributions do not include send-email in a
  250. default install of git; you may need to download additional packages,
  251. such as 'git-email' on Fedora-based systems.) Patch series need a cover
  252. letter, with shallow threading (all patches in the series are
  253. in-reply-to the cover letter, but not to each other); single unrelated
  254. patches do not need a cover letter (but if you do send a cover letter,
  255. use ``--numbered`` so the cover and the patch have distinct subject lines).
  256. Patches are easier to find if they start a new top-level thread, rather
  257. than being buried in-reply-to another existing thread.
  258. .. _avoid_posting_large_binary_blob:
  259. Avoid posting large binary blob
  260. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  261. If you added binaries to the repository, consider producing the patch
  262. emails using ``git format-patch --no-binary`` and include a link to a
  263. git repository to fetch the original commit.
  264. .. _patch_emails_must_include_a_signed_off_by_line:
  265. Patch emails must include a ``Signed-off-by:`` line
  266. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  267. Your patches **must** include a Signed-off-by: line. This is a hard
  268. requirement because it's how you say "I'm legally okay to contribute
  269. this and happy for it to go into QEMU". The process is modelled after
  270. the `Linux kernel
  271. <http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297>`__
  272. policy.
  273. If you wrote the patch, make sure your "From:" and "Signed-off-by:"
  274. lines use the same spelling. It's okay if you subscribe or contribute to
  275. the list via more than one address, but using multiple addresses in one
  276. commit just confuses things. If someone else wrote the patch, git will
  277. include a "From:" line in the body of the email (different from your
  278. envelope From:) that will give credit to the correct author; but again,
  279. that author's Signed-off-by: line is mandatory, with the same spelling.
  280. The name used with "Signed-off-by" does not need to be your legal name,
  281. nor birth name, nor appear on any government ID. It is the identity you
  282. choose to be known by in the community, but should not be anonymous,
  283. nor misrepresent whom you are.
  284. There are various tooling options for automatically adding these tags
  285. include using ``git commit -s`` or ``git format-patch -s``. For more
  286. information see `SubmittingPatches 1.12
  287. <http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n297>`__.
  288. .. _include_a_meaningful_cover_letter:
  289. Include a meaningful cover letter
  290. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  291. This is a requirement for any series with multiple patches (as it aids
  292. continuous integration), but optional for an isolated patch. The cover
  293. letter explains the overall goal of such a series, and also provides a
  294. convenient 0/N email for others to reply to the series as a whole. A
  295. one-time setup of ``git config format.coverletter auto`` (refer to
  296. `git-config <http://git-scm.com/docs/git-config>`__) will generate the
  297. cover letter as needed.
  298. When reviewers don't know your goal at the start of their review, they
  299. may object to early changes that don't make sense until the end of the
  300. series, because they do not have enough context yet at that point of
  301. their review. A series where the goal is unclear also risks a higher
  302. number of review-fix cycles because the reviewers haven't bought into
  303. the idea yet. If the cover letter can explain these points to the
  304. reviewer, the process will be smoother patches will get merged faster.
  305. Make sure your cover letter includes a diffstat of changes made over the
  306. entire series; potential reviewers know what files they are interested
  307. in, and they need an easy way determine if your series touches them.
  308. .. _use_the_rfc_tag_if_needed:
  309. Use the RFC tag if needed
  310. ~~~~~~~~~~~~~~~~~~~~~~~~~
  311. For example, "[PATCH RFC v2]". ``git format-patch --subject-prefix=RFC``
  312. can help.
  313. "RFC" means "Request For Comments" and is a statement that you don't
  314. intend for your patchset to be applied to master, but would like some
  315. review on it anyway. Reasons for doing this include:
  316. - the patch depends on some pending kernel changes which haven't yet
  317. been accepted, so the QEMU patch series is blocked until that
  318. dependency has been dealt with, but is worth reviewing anyway
  319. - the patch set is not finished yet (perhaps it doesn't cover all use
  320. cases or work with all targets) but you want early review of a major
  321. API change or design structure before continuing
  322. In general, since it's asking other people to do review work on a
  323. patchset that the submitter themselves is saying shouldn't be applied,
  324. it's best to:
  325. - use it sparingly
  326. - in the cover letter, be clear about why a patch is an RFC, what areas
  327. of the patchset you're looking for review on, and why reviewers
  328. should care
  329. .. _consider_whether_your_patch_is_applicable_for_stable:
  330. Consider whether your patch is applicable for stable
  331. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  332. If your patch fixes a severe issue or a regression, it may be applicable
  333. for stable. In that case, consider adding ``Cc: qemu-stable@nongnu.org``
  334. to your patch to notify the stable maintainers.
  335. For more details on how QEMU's stable process works, refer to the
  336. :ref:`stable-process` page.
  337. .. _participating_in_code_review:
  338. Retrieve an existing series
  339. ---------------------------
  340. If you want to apply an existing series on top of your tree, you can simply use
  341. `b4 <https://github.com/mricon/b4>`__.
  342. ::
  343. b4 shazam $msg-id
  344. The message id is related to the patch series that has been sent to the mailing
  345. list. You need to retrieve the "Message-Id:" header from one of the patches. Any
  346. of them can be used and b4 will apply the whole series.
  347. Participating in Code Review
  348. ----------------------------
  349. All patches submitted to the QEMU project go through a code review
  350. process before they are accepted. This will often mean a series will
  351. go through a number of iterations before being picked up by
  352. :ref:`maintainers<maintainers>`. You therefore should be prepared to
  353. read replies to your messages and be willing to act on them.
  354. Maintainers are often willing to manually fix up first-time
  355. contributions, since there is a learning curve involved in making an
  356. ideal patch submission. However for the best results you should
  357. proactively respond to suggestions with changes or justifications for
  358. your current approach.
  359. Some areas of code that are well maintained may review patches
  360. quickly, lesser-loved areas of code may have a longer delay.
  361. .. _stay_around_to_fix_problems_raised_in_code_review:
  362. Stay around to fix problems raised in code review
  363. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  364. Not many patches get into QEMU straight away -- it is quite common that
  365. developers will identify bugs, or suggest a cleaner approach, or even
  366. just point out code style issues or commit message typos. You'll need to
  367. respond to these, and then send a second version of your patches with
  368. the issues fixed. This takes a little time and effort on your part, but
  369. if you don't do it then your changes will never get into QEMU.
  370. Remember that a maintainer is under no obligation to take your
  371. patches. If someone has spent the time reviewing your code and
  372. suggesting improvements and you simply re-post without either
  373. addressing the comment directly or providing additional justification
  374. for the change then it becomes wasted effort. You cannot demand others
  375. merge and then fix up your code after the fact.
  376. When replying to comments on your patches **reply to all and not just
  377. the sender** -- keeping discussion on the mailing list means everybody
  378. can follow it. Remember the spirit of the :ref:`code_of_conduct` and
  379. keep discussions respectful and collaborative and avoid making
  380. personal comments.
  381. .. _pay_attention_to_review_comments:
  382. Pay attention to review comments
  383. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  384. Someone took their time to review your work, and it pays to respect that
  385. effort; repeatedly submitting a series without addressing all comments
  386. from the previous round tends to alienate reviewers and stall your
  387. patch. Reviewers aren't always perfect, so it is okay if you want to
  388. argue that your code was correct in the first place instead of blindly
  389. doing everything the reviewer asked. On the other hand, if someone
  390. pointed out a potential issue during review, then even if your code
  391. turns out to be correct, it's probably a sign that you should improve
  392. your commit message and/or comments in the code explaining why the code
  393. is correct.
  394. If you fix issues that are raised during review **resend the entire
  395. patch series** not just the one patch that was changed. This allows
  396. maintainers to easily apply the fixed series without having to manually
  397. identify which patches are relevant. Send the new version as a complete
  398. fresh email or series of emails -- don't try to make it a followup to
  399. version 1. (This helps automatic patch email handling tools distinguish
  400. between v1 and v2 emails.)
  401. .. _when_resending_patches_add_a_version_tag:
  402. When resending patches add a version tag
  403. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  404. All patches beyond the first version should include a version tag -- for
  405. example, "[PATCH v2]". This means people can easily identify whether
  406. they're looking at the most recent version. (The first version of a
  407. patch need not say "v1", just [PATCH] is sufficient.) For patch series,
  408. the version applies to the whole series -- even if you only change one
  409. patch, you resend the entire series and mark it as "v2". Don't try to
  410. track versions of different patches in the series separately. `git
  411. format-patch <http://git-scm.com/docs/git-format-patch>`__ and `git
  412. send-email <http://git-scm.com/docs/git-send-email>`__ both understand
  413. the ``-v2`` option to make this easier. Send each new revision as a new
  414. top-level thread, rather than burying it in-reply-to an earlier
  415. revision, as many reviewers are not looking inside deep threads for new
  416. patches.
  417. .. _include_version_history_in_patchset_revisions:
  418. Include version history in patchset revisions
  419. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  420. For later versions of patches, include a summary of changes from
  421. previous versions, but not in the commit message itself. In an email
  422. formatted as a git patch, the commit message is the part above the ``---``
  423. line, and this will go into the git changelog when the patch is
  424. committed. This part should be a self-contained description of what this
  425. version of the patch does, written to make sense to anybody who comes
  426. back to look at this commit in git in six months' time. The part below
  427. the ``---`` line and above the patch proper (git format-patch puts the
  428. diffstat here) is a good place to put remarks for people reading the
  429. patch email, and this is where the "changes since previous version"
  430. summary belongs. The `git-publish
  431. <https://github.com/stefanha/git-publish>`__ script can help with
  432. tracking a good summary across versions. Also, the `git-backport-diff
  433. <https://github.com/codyprime/git-scripts>`__ script can help focus
  434. reviewers on what changed between revisions.
  435. .. _tips_and_tricks:
  436. Tips and Tricks
  437. ---------------
  438. .. _proper_use_of_reviewed_by_tags_can_aid_review:
  439. Proper use of Reviewed-by: tags can aid review
  440. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  441. When reviewing a large series, a reviewer can reply to some of the
  442. patches with a Reviewed-by tag, stating that they are happy with that
  443. patch in isolation (sometimes conditional on minor cleanup, like fixing
  444. whitespace, that doesn't affect code content). You should then update
  445. those commit messages by hand to include the Reviewed-by tag, so that in
  446. the next revision, reviewers can spot which patches were already clean
  447. from the previous round. Conversely, if you significantly modify a patch
  448. that was previously reviewed, remove the reviewed-by tag out of the
  449. commit message, as well as listing the changes from the previous
  450. version, to make it easier to focus a reviewer's attention to your
  451. changes.
  452. .. _if_your_patch_seems_to_have_been_ignored:
  453. If your patch seems to have been ignored
  454. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  455. If your patchset has received no replies you should "ping" it after a
  456. week or two, by sending an email as a reply-to-all to the patch mail,
  457. including the word "ping" and ideally also a link to the page for the
  458. patch on `patchew <https://patchew.org/QEMU/>`__ or
  459. `lore.kernel.org <https://lore.kernel.org/qemu-devel/>`__. It's worth
  460. double-checking for reasons why your patch might have been ignored
  461. (forgot to CC the maintainer? annoyed people by failing to respond to
  462. review comments on an earlier version?), but often for less-maintained
  463. areas of QEMU patches do just slip through the cracks. If your ping is
  464. also ignored, ping again after another week or so. As the submitter, you
  465. are the person with the most motivation to get your patch applied, so
  466. you have to be persistent.
  467. .. _is_my_patch_in:
  468. Is my patch in?
  469. ~~~~~~~~~~~~~~~
  470. QEMU has some Continuous Integration machines that try to catch patch
  471. submission problems as soon as possible. `patchew
  472. <http://patchew.org/QEMU/>`__ includes a web interface for tracking the
  473. status of various threads that have been posted to the list, and may
  474. send you an automated mail if it detected a problem with your patch.
  475. Once your patch has had enough review on list, the maintainer for that
  476. area of code will send notification to the list that they are including
  477. your patch in a particular staging branch. Periodically, the maintainer
  478. then takes care of :ref:`submitting-a-pull-request`
  479. for aggregating topic branches into mainline QEMU. Generally, you do not
  480. need to send a pull request unless you have contributed enough patches
  481. to become a maintainer over a particular section of code. Maintainers
  482. may further modify your commit, by resolving simple merge conflicts or
  483. fixing minor typos pointed out during review, but will always add a
  484. Signed-off-by line in addition to yours, indicating that it went through
  485. their tree. Occasionally, the maintainer's pull request may hit more
  486. difficult merge conflicts, where you may be requested to help rebase and
  487. resolve the problems. It may take a couple of weeks between when your
  488. patch first had a positive review to when it finally lands in qemu.git;
  489. release cycle freezes may extend that time even longer.
  490. .. _return_the_favor:
  491. Return the favor
  492. ~~~~~~~~~~~~~~~~
  493. Peer review only works if everyone chips in a bit of review time. If
  494. everyone submitted more patches than they reviewed, we would have a
  495. patch backlog. A good goal is to try to review at least as many patches
  496. from others as what you submit. Don't worry if you don't know the code
  497. base as well as a maintainer; it's perfectly fine to admit when your
  498. review is weak because you are unfamiliar with the code.