SubmittingPatches: itemize and reflect upon well written changes
[git/git.git] / Documentation / SubmittingPatches
CommitLineData
56333bac
JS
1Checklist (and a short version for the impatient):
2
a7af09d2
JA
3 Commits:
4
56333bac
JS
5 - make commits of logical units
6 - check for unnecessary whitespace with "git diff --check"
7 before committing
8 - do not check in commented out code or unneeded files
56333bac
JS
9 - the first line of the commit message should be a short
10 description and should skip the full stop
47afed5d
SV
11 - the body should provide a meaningful commit message, which:
12 - uses the imperative, present tense: "change",
13 not "changed" or "changes".
14 - includes motivation for the change, and contrasts
15 its implementation with previous behaviour
56333bac 16 - if you want your work included in git.git, add a
8e7425da 17 "Signed-off-by: Your Name <you@example.com>" line to the
56333bac
JS
18 commit message (or just use the option "-s" when
19 committing) to confirm that you agree to the Developer's
20 Certificate of Origin
d3017e93
JS
21 - make sure that you have tests for the bug you are fixing
22 - make sure that the test suite passes after your commit
a7af09d2
JA
23
24 Patch:
25
56333bac 26 - use "git format-patch -M" to create the patch
a7af09d2 27 - do not PGP sign your patch
56333bac
JS
28 - do not attach your patch, but read in the mail
29 body, unless you cannot teach your mailer to
30 leave the formatting of the patch alone.
31 - be careful doing cut & paste into your mailer, not to
32 corrupt whitespaces.
33 - provide additional information (which is unsuitable for
34 the commit message) between the "---" and the diffstat
15320175
AR
35 - if you change, add, or remove a command line option or
36 make some other user interface change, the associated
37 documentation should be updated as well.
d3017e93
JS
38 - if your name is not writable in ASCII, make sure that
39 you send off a message in the correct encoding.
13d4e6f7 40 - send the patch to the list (git@vger.kernel.org) and the
0b059940
JH
41 maintainer (gitster@pobox.com) if (and only if) the patch
42 is ready for inclusion. If you use git-send-email(1),
43 please test it first by sending email to yourself.
56333bac
JS
44
45Long version:
46
31408251
JH
47I started reading over the SubmittingPatches document for Linux
48kernel, primarily because I wanted to have a document similar to
49it for the core GIT to make sure people understand what they are
50doing when they write "Signed-off-by" line.
51
52But the patch submission requirements are a lot more relaxed
45d2b286
JH
53here on the technical/contents front, because the core GIT is
54thousand times smaller ;-). So here is only the relevant bits.
31408251
JH
55
56
57(1) Make separate commits for logically separate changes.
58
59Unless your patch is really trivial, you should not be sending
60out a patch that was generated between your working tree and
61your commit head. Instead, always make a commit with complete
62commit message and generate a series of patches from your
63repository. It is a good discipline.
64
65Describe the technical detail of the change(s).
66
45d2b286 67If your description starts to get too long, that's a sign that you
31408251 68probably need to split up your commit to finer grained pieces.
47afed5d
SV
69That being said, patches which plainly describe the things that
70help reviewers check the patch, and future maintainers understand
71the code, are the most beautiful patches. Descriptions that summarise
72the point in the subject well, and describe the motivation for the
73change, the approach taken by the change, and if relevant how this
74differs substantially from the prior version, can be found on Usenet
75archives back into the late 80's. Consider it like good Netiquette,
76but for code.
31408251 77
45d2b286
JH
78Oh, another thing. I am picky about whitespaces. Make sure your
79changes do not trigger errors with the sample pre-commit hook shipped
16507fcf
BL
80in templates/hooks--pre-commit. To help ensure this does not happen,
81run git diff --check on your changes before you commit.
31408251 82
31408251 83
243bfd33
JS
84(1a) Try to be nice to older C compilers
85
8b1d88e8 86We try to support a wide range of C compilers to compile
243bfd33
JS
87git with. That means that you should not use C99 initializers, even
88if a lot of compilers grok it.
89
90Also, variables have to be declared at the beginning of the block
91(you can check this with gcc, using the -Wdeclaration-after-statement
92option).
93
94Another thing: NULL pointers shall be written as NULL, not as 0.
95
96
45d2b286
JH
97(2) Generate your patch using git tools out of your commits.
98
99git based diff tools (git, Cogito, and StGIT included) generate
100unidiff which is the preferred format.
101
31408251
JH
102You do not have to be afraid to use -M option to "git diff" or
103"git format-patch", if your patch involves file renames. The
104receiving end can handle them just fine.
105
106Please make sure your patch does not include any extra files
107which do not belong in a patch submission. Make sure to review
108your patch after generating it, to ensure accuracy. Before
109sending out, please make sure it cleanly applies to the "master"
45d2b286
JH
110branch head. If you are preparing a work based on "next" branch,
111that is fine, but please mark it as such.
31408251
JH
112
113
114(3) Sending your patches.
115
45d2b286 116People on the git mailing list need to be able to read and
31408251
JH
117comment on the changes you are submitting. It is important for
118a developer to be able to "quote" your changes, using standard
119e-mail tools, so that they may comment on specific portions of
addf88e4 120your code. For this reason, all patches should be submitted
45d2b286
JH
121"inline". WARNING: Be wary of your MUAs word-wrap
122corrupting your patch. Do not cut-n-paste your patch; you can
123lose tabs that way if you are not careful.
31408251 124
45d2b286 125It is a common convention to prefix your subject line with
31408251 126[PATCH]. This lets people easily distinguish patches from other
4e891acf
JH
127e-mail discussions. Use of additional markers after PATCH and
128the closing bracket to mark the nature of the patch is also
129encouraged. E.g. [PATCH/RFC] is often used when the patch is
130not ready to be applied but it is for discussion, [PATCH v2],
131[PATCH v3] etc. are often seen when you are sending an update to
132what you have previously sent.
31408251
JH
133
134"git format-patch" command follows the best current practice to
135format the body of an e-mail message. At the beginning of the
136patch should come your commit message, ending with the
137Signed-off-by: lines, and a line that consists of three dashes,
138followed by the diffstat information and the patch itself. If
139you are forwarding a patch from somebody else, optionally, at
140the beginning of the e-mail message just before the commit
141message starts, you can put a "From: " line to name that person.
142
143You often want to add additional explanation about the patch,
144other than the commit message itself. Place such "cover letter"
145material between the three dash lines and the diffstat.
146
147Do not attach the patch as a MIME attachment, compressed or not.
e30b217b
JH
148Do not let your e-mail client send quoted-printable. Do not let
149your e-mail client send format=flowed which would destroy
150whitespaces in your patches. Many
31408251
JH
151popular e-mail applications will not always transmit a MIME
152attachment as plain text, making it impossible to comment on
153your code. A MIME attachment also takes a bit more time to
154process. This does not decrease the likelihood of your
155MIME-attached change being accepted, but it makes it more likely
156that it will be postponed.
157
158Exception: If your mailer is mangling patches then someone may ask
9847f7e0 159you to re-send them using MIME, that is OK.
31408251 160
9847f7e0
JH
161Do not PGP sign your patch, at least for now. Most likely, your
162maintainer or other people on the list would not have your PGP
163key and would not bother obtaining it anyway. Your patch is not
164judged by who you are; a good patch from an unknown origin has a
165far better chance of being accepted than a patch from a known,
166respected origin that is done poorly or does incorrect things.
167
168If you really really really really want to do a PGP signed
169patch, format it as "multipart/signed", not a text/plain message
170that starts with '-----BEGIN PGP SIGNED MESSAGE-----'. That is
171not a text/plain, it's something else.
172
173Note that your maintainer does not necessarily read everything
174on the git mailing list. If your patch is for discussion first,
175send it "To:" the mailing list, and optionally "cc:" him. If it
176is trivially correct or after the list reached a consensus, send
0b059940
JH
177it "To:" the maintainer and optionally "cc:" the list for
178inclusion.
31408251 179
04d24455
JH
180Also note that your maintainer does not actively involve himself in
181maintaining what are in contrib/ hierarchy. When you send fixes and
182enhancements to them, do not forget to "cc: " the person who primarily
183worked on that hierarchy in contrib/.
184
31408251 185
84ab7b6f 186(4) Sign your work
31408251
JH
187
188To improve tracking of who did what, we've borrowed the
189"sign-off" procedure from the Linux kernel project on patches
190that are being emailed around. Although core GIT is a lot
191smaller project it is a good discipline to follow it.
192
193The sign-off is a simple line at the end of the explanation for
194the patch, which certifies that you wrote it or otherwise have
195the right to pass it on as a open-source patch. The rules are
196pretty simple: if you can certify the below:
197
198 Developer's Certificate of Origin 1.1
199
200 By making a contribution to this project, I certify that:
201
202 (a) The contribution was created in whole or in part by me and I
203 have the right to submit it under the open source license
204 indicated in the file; or
205
206 (b) The contribution is based upon previous work that, to the best
207 of my knowledge, is covered under an appropriate open source
208 license and I have the right under that license to submit that
209 work with modifications, whether created in whole or in part
210 by me, under the same open source license (unless I am
211 permitted to submit under a different license), as indicated
212 in the file; or
213
214 (c) The contribution was provided directly to me by some other
215 person who certified (a), (b) or (c) and I have not modified
216 it.
217
218 (d) I understand and agree that this project and the contribution
219 are public and that a record of the contribution (including all
220 personal information I submit with it, including my sign-off) is
221 maintained indefinitely and may be redistributed consistent with
222 this project or the open source license(s) involved.
223
224then you just add a line saying
225
226 Signed-off-by: Random J Developer <random@developer.example.org>
227
69945602
PC
228This line can be automatically added by git if you run the git-commit
229command with the -s option.
230
c11c3b56
JH
231Notice that you can place your own Signed-off-by: line when
232forwarding somebody else's patch with the above rules for
233D-C-O. Indeed you are encouraged to do so. Do not forget to
234place an in-body "From: " line at the beginning to properly attribute
235the change to its true author (see (2) above).
236
67275247
MV
237Also notice that a real name is used in the Signed-off-by: line. Please
238don't hide your real name.
239
c11c3b56
JH
240Some people also put extra tags at the end.
241
242"Acked-by:" says that the patch was reviewed by the person who
243is more familiar with the issues and the area the patch attempts
244to modify. "Tested-by:" says the patch was tested by the person
245and found to have the desired effect.
9740d289 246
a941fb4a
JH
247------------------------------------------------
248An ideal patch flow
249
250Here is an ideal patch flow for this project the current maintainer
251suggests to the contributors:
252
253 (0) You come up with an itch. You code it up.
254
255 (1) Send it to the list and cc people who may need to know about
256 the change.
257
258 The people who may need to know are the ones whose code you
259 are butchering. These people happen to be the ones who are
260 most likely to be knowledgeable enough to help you, but
261 they have no obligation to help you (i.e. you ask for help,
262 don't demand). "git log -p -- $area_you_are_modifying" would
263 help you find out who they are.
264
265 (2) You get comments and suggestions for improvements. You may
266 even get them in a "on top of your change" patch form.
267
268 (3) Polish, refine, and re-send to the list and the people who
269 spend their time to improve your patch. Go back to step (2).
270
271 (4) The list forms consensus that the last round of your patch is
272 good. Send it to the list and cc the maintainer.
273
274 (5) A topic branch is created with the patch and is merged to 'next',
275 and cooked further and eventually graduates to 'master'.
276
277In any time between the (2)-(3) cycle, the maintainer may pick it up
278from the list and queue it to 'pu', in order to make it easier for
279people play with it without having to pick up and apply the patch to
280their trees themselves.
281
9740d289
JH
282------------------------------------------------
283MUA specific hints
284
285Some of patches I receive or pick up from the list share common
286patterns of breakage. Please make sure your MUA is set up
287properly not to corrupt whitespaces. Here are two common ones
288I have seen:
289
290* Empty context lines that do not have _any_ whitespace.
291
292* Non empty context lines that have one extra whitespace at the
293 beginning.
294
9847f7e0
JH
295One test you could do yourself if your MUA is set up correctly is:
296
297* Send the patch to yourself, exactly the way you would, except
298 To: and Cc: lines, which would not contain the list and
299 maintainer address.
300
301* Save that patch to a file in UNIX mailbox format. Call it say
302 a.patch.
303
304* Try to apply to the tip of the "master" branch from the
305 git.git public repository:
306
307 $ git fetch http://kernel.org/pub/scm/git/git.git master:test-apply
308 $ git checkout test-apply
309 $ git reset --hard
59c8e2cb 310 $ git am a.patch
9847f7e0
JH
311
312If it does not apply correctly, there can be various reasons.
313
314* Your patch itself does not apply cleanly. That is _bad_ but
315 does not have much to do with your MUA. Please rebase the
316 patch appropriately.
317
59c8e2cb 318* Your MUA corrupted your patch; "am" would complain that
51ef1daa 319 the patch does not apply. Look at .git/rebase-apply/ subdirectory and
9847f7e0
JH
320 see what 'patch' file contains and check for the common
321 corruption patterns mentioned above.
322
323* While you are at it, check what are in 'info' and
324 'final-commit' files as well. If what is in 'final-commit' is
325 not exactly what you would want to see in the commit log
326 message, it is very likely that your maintainer would end up
327 hand editing the log message when he applies your patch.
328 Things like "Hi, this is my first patch.\n", if you really
329 want to put in the patch e-mail, should come after the
330 three-dash line that signals the end of the commit message.
331
9740d289
JH
332
333Pine
334----
335
336(Johannes Schindelin)
337
338I don't know how many people still use pine, but for those poor
339souls it may be good to mention that the quell-flowed-text is
340needed for recent versions.
341
342... the "no-strip-whitespace-before-send" option, too. AFAIK it
343was introduced in 4.60.
344
345(Linus Torvalds)
346
347And 4.58 needs at least this.
348
349---
350diff-tree 8326dd8350be64ac7fc805f6563a1d61ad10d32c (from e886a61f76edf5410573e92e38ce22974f9c40f1)
351Author: Linus Torvalds <torvalds@g5.osdl.org>
352Date: Mon Aug 15 17:23:51 2005 -0700
353
354 Fix pine whitespace-corruption bug
355
356 There's no excuse for unconditionally removing whitespace from
357 the pico buffers on close.
358
359diff --git a/pico/pico.c b/pico/pico.c
360--- a/pico/pico.c
361+++ b/pico/pico.c
362@@ -219,7 +219,9 @@ PICO *pm;
a6080a0a
JH
363 switch(pico_all_done){ /* prepare for/handle final events */
364 case COMP_EXIT : /* already confirmed */
365 packheader();
9740d289 366+#if 0
a6080a0a 367 stripwhitespace();
9740d289 368+#endif
a6080a0a
JH
369 c |= COMP_EXIT;
370 break;
371
9740d289 372
1eb446fa
JH
373(Daniel Barkalow)
374
375> A patch to SubmittingPatches, MUA specific help section for
376> users of Pine 4.63 would be very much appreciated.
377
378Ah, it looks like a recent version changed the default behavior to do the
379right thing, and inverted the sense of the configuration option. (Either
380that or Gentoo did it.) So you need to set the
381"no-strip-whitespace-before-send" option, unless the option you have is
382"strip-whitespace-before-send", in which case you should avoid checking
383it.
384
9740d289
JH
385
386Thunderbird
387-----------
388
389(A Large Angry SCM)
390
1a526d48
JW
391By default, Thunderbird will both wrap emails as well as flag them as
392being 'format=flowed', both of which will make the resulting email unusable
393by git.
394
9740d289 395Here are some hints on how to successfully submit patches inline using
cf6de18a 396Thunderbird.
9740d289 397
1a526d48
JW
398There are two different approaches. One approach is to configure
399Thunderbird to not mangle patches. The second approach is to use
400an external editor to keep Thunderbird from mangling the patches.
401
402Approach #1 (configuration):
403
404This recipe is current as of Thunderbird 2.0.0.19. Three steps:
405 1. Configure your mail server composition as plain text
406 Edit...Account Settings...Composition & Addressing,
407 uncheck 'Compose Messages in HTML'.
408 2. Configure your general composition window to not wrap
409 Edit..Preferences..Composition, wrap plain text messages at 0
410 3. Disable the use of format=flowed
411 Edit..Preferences..Advanced..Config Editor. Search for:
412 mailnews.send_plaintext_flowed
413 toggle it to make sure it is set to 'false'.
414
415After that is done, you should be able to compose email as you
416otherwise would (cut + paste, git-format-patch | git-imap-send, etc),
417and the patches should not be mangled.
418
419Approach #2 (external editor):
420
9740d289
JH
421This recipe appears to work with the current [*1*] Thunderbird from Suse.
422
423The following Thunderbird extensions are needed:
424 AboutConfig 0.5
425 http://aboutconfig.mozdev.org/
ff62b7f3
LS
426 External Editor 0.7.2
427 http://globs.org/articles.php?lng=en&pg=8
9740d289
JH
428
4291) Prepare the patch as a text file using your method of choice.
430
4312) Before opening a compose window, use Edit->Account Settings to
432uncheck the "Compose messages in HTML format" setting in the
433"Composition & Addressing" panel of the account to be used to send the
434patch. [*2*]
435
4363) In the main Thunderbird window, _before_ you open the compose window
437for the patch, use Tools->about:config to set the following to the
438indicated values:
439 mailnews.send_plaintext_flowed => false
cf6de18a 440 mailnews.wraplength => 0
9740d289
JH
441
4424) Open a compose window and click the external editor icon.
443
4445) In the external editor window, read in the patch file and exit the
445editor normally.
446
4476) Back in the compose window: Add whatever other text you wish to the
448message, complete the addressing and subject fields, and press send.
449
4507) Optionally, undo the about:config/account settings changes made in
451steps 2 & 3.
452
453
454[Footnotes]
455*1* Version 1.0 (20041207) from the MozillaThunderbird-1.0-5 rpm of Suse
4569.3 professional updates.
457
458*2* It may be possible to do this with about:config and the following
459settings but I haven't tried, yet.
460 mail.html_compose => false
461 mail.identity.default.compose_html => false
462 mail.identity.id?.compose_html => false
463
0c3d26d2
LS
464(Lukas Sandström)
465
466There is a script in contrib/thunderbird-patch-inline which can help
467you include patches with Thunderbird in an easy way. To use it, do the
468steps above and then use the script as the external editor.
e30b217b 469
e30b217b
JH
470Gnus
471----
472
473'|' in the *Summary* buffer can be used to pipe the current
474message to an external program, and this is a handy way to drive
475"git am". However, if the message is MIME encoded, what is
476piped into the program is the representation you see in your
477*Article* buffer after unwrapping MIME. This is often not what
478you would want for two reasons. It tends to screw up non ASCII
479characters (most notably in people's names), and also
480whitespaces (fatal in patches). Running 'C-u g' to display the
481message in raw form before using '|' to run the pipe can work
482this problem around.
483
451fd65a
MB
484
485KMail
486-----
487
488This should help you to submit patches inline using KMail.
489
4901) Prepare the patch as a text file.
491
4922) Click on New Mail.
493
4943) Go under "Options" in the Composer window and be sure that
495"Word wrap" is not set.
496
4974) Use Message -> Insert file... and insert the patch.
498
4995) Back in the compose window: add whatever other text you wish to the
500message, complete the addressing and subject fields, and press send.
c2163c6a
TPW
501
502
503Gmail
504-----
505
50dffd4e
JT
506GMail does not appear to have any way to turn off line wrapping in the web
507interface, so this will mangle any emails that you send. You can however
508use any IMAP email client to connect to the google imap server, and forward
509the emails through that. Just make sure to disable line wrapping in that
510email client. Alternatively, use "git send-email" instead.
511
c2163c6a
TPW
512Submitting properly formatted patches via Gmail is simple now that
513IMAP support is available. First, edit your ~/.gitconfig to specify your
514account settings:
515
516[imap]
517 folder = "[Gmail]/Drafts"
518 host = imaps://imap.gmail.com
519 user = user@gmail.com
520 pass = p4ssw0rd
521 port = 993
522 sslverify = false
523
50dffd4e
JT
524You might need to instead use: folder = "[Google Mail]/Drafts" if you get an error
525that the "Folder doesn't exist".
526
c2163c6a
TPW
527Next, ensure that your Gmail settings are correct. In "Settings" the
528"Use Unicode (UTF-8) encoding for outgoing messages" should be checked.
529
530Once your commits are ready to send to the mailing list, run the following
531command to send the patch emails to your Gmail Drafts folder.
532
533 $ git format-patch -M --stdout origin/master | git imap-send
534
535Go to your Gmail account, open the Drafts folder, find the patch email, fill
536in the To: and CC: fields and send away!
50dffd4e 537