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