Johannes Schindelin | 56333ba | 2007-03-05 16:37:54 +0100 | [diff] [blame] | 1 | Checklist (and a short version for the impatient): |
| 2 | |
| 3 | - make commits of logical units |
| 4 | - check for unnecessary whitespace with "git diff --check" |
| 5 | before committing |
| 6 | - do not check in commented out code or unneeded files |
| 7 | - provide a meaningful commit message |
| 8 | - the first line of the commit message should be a short |
| 9 | description and should skip the full stop |
| 10 | - if you want your work included in git.git, add a |
| 11 | "Signed-off-by: Your Name <your@email.com>" line to the |
| 12 | commit message (or just use the option "-s" when |
| 13 | committing) to confirm that you agree to the Developer's |
| 14 | Certificate of Origin |
| 15 | - do not PGP sign your patch |
| 16 | - use "git format-patch -M" to create the patch |
| 17 | - do not attach your patch, but read in the mail |
| 18 | body, unless you cannot teach your mailer to |
| 19 | leave the formatting of the patch alone. |
| 20 | - be careful doing cut & paste into your mailer, not to |
| 21 | corrupt whitespaces. |
| 22 | - provide additional information (which is unsuitable for |
| 23 | the commit message) between the "---" and the diffstat |
| 24 | - send the patch to the list _and_ the maintainer |
| 25 | |
| 26 | Long version: |
| 27 | |
Junio C Hamano | 3140825 | 2005-08-12 23:48:09 -0700 | [diff] [blame] | 28 | I started reading over the SubmittingPatches document for Linux |
| 29 | kernel, primarily because I wanted to have a document similar to |
| 30 | it for the core GIT to make sure people understand what they are |
| 31 | doing when they write "Signed-off-by" line. |
| 32 | |
| 33 | But the patch submission requirements are a lot more relaxed |
Junio C Hamano | 45d2b28 | 2006-02-17 16:15:26 -0800 | [diff] [blame] | 34 | here on the technical/contents front, because the core GIT is |
| 35 | thousand times smaller ;-). So here is only the relevant bits. |
Junio C Hamano | 3140825 | 2005-08-12 23:48:09 -0700 | [diff] [blame] | 36 | |
| 37 | |
| 38 | (1) Make separate commits for logically separate changes. |
| 39 | |
| 40 | Unless your patch is really trivial, you should not be sending |
| 41 | out a patch that was generated between your working tree and |
| 42 | your commit head. Instead, always make a commit with complete |
| 43 | commit message and generate a series of patches from your |
| 44 | repository. It is a good discipline. |
| 45 | |
| 46 | Describe the technical detail of the change(s). |
| 47 | |
Junio C Hamano | 45d2b28 | 2006-02-17 16:15:26 -0800 | [diff] [blame] | 48 | If your description starts to get too long, that's a sign that you |
Junio C Hamano | 3140825 | 2005-08-12 23:48:09 -0700 | [diff] [blame] | 49 | probably need to split up your commit to finer grained pieces. |
| 50 | |
Junio C Hamano | 45d2b28 | 2006-02-17 16:15:26 -0800 | [diff] [blame] | 51 | Oh, another thing. I am picky about whitespaces. Make sure your |
| 52 | changes do not trigger errors with the sample pre-commit hook shipped |
Bill Lear | 16507fc | 2007-01-27 07:21:53 -0600 | [diff] [blame] | 53 | in templates/hooks--pre-commit. To help ensure this does not happen, |
| 54 | run git diff --check on your changes before you commit. |
Junio C Hamano | 3140825 | 2005-08-12 23:48:09 -0700 | [diff] [blame] | 55 | |
Junio C Hamano | 3140825 | 2005-08-12 23:48:09 -0700 | [diff] [blame] | 56 | |
Junio C Hamano | 45d2b28 | 2006-02-17 16:15:26 -0800 | [diff] [blame] | 57 | (2) Generate your patch using git tools out of your commits. |
| 58 | |
| 59 | git based diff tools (git, Cogito, and StGIT included) generate |
| 60 | unidiff which is the preferred format. |
| 61 | |
Junio C Hamano | 3140825 | 2005-08-12 23:48:09 -0700 | [diff] [blame] | 62 | You do not have to be afraid to use -M option to "git diff" or |
| 63 | "git format-patch", if your patch involves file renames. The |
| 64 | receiving end can handle them just fine. |
| 65 | |
| 66 | Please make sure your patch does not include any extra files |
| 67 | which do not belong in a patch submission. Make sure to review |
| 68 | your patch after generating it, to ensure accuracy. Before |
| 69 | sending out, please make sure it cleanly applies to the "master" |
Junio C Hamano | 45d2b28 | 2006-02-17 16:15:26 -0800 | [diff] [blame] | 70 | branch head. If you are preparing a work based on "next" branch, |
| 71 | that is fine, but please mark it as such. |
Junio C Hamano | 3140825 | 2005-08-12 23:48:09 -0700 | [diff] [blame] | 72 | |
| 73 | |
| 74 | (3) Sending your patches. |
| 75 | |
Junio C Hamano | 45d2b28 | 2006-02-17 16:15:26 -0800 | [diff] [blame] | 76 | People on the git mailing list need to be able to read and |
Junio C Hamano | 3140825 | 2005-08-12 23:48:09 -0700 | [diff] [blame] | 77 | comment on the changes you are submitting. It is important for |
| 78 | a developer to be able to "quote" your changes, using standard |
| 79 | e-mail tools, so that they may comment on specific portions of |
Pavel Roskin | addf88e | 2006-07-09 03:44:30 -0400 | [diff] [blame] | 80 | your code. For this reason, all patches should be submitted |
Junio C Hamano | 45d2b28 | 2006-02-17 16:15:26 -0800 | [diff] [blame] | 81 | "inline". WARNING: Be wary of your MUAs word-wrap |
| 82 | corrupting your patch. Do not cut-n-paste your patch; you can |
| 83 | lose tabs that way if you are not careful. |
Junio C Hamano | 3140825 | 2005-08-12 23:48:09 -0700 | [diff] [blame] | 84 | |
Junio C Hamano | 45d2b28 | 2006-02-17 16:15:26 -0800 | [diff] [blame] | 85 | It is a common convention to prefix your subject line with |
Junio C Hamano | 3140825 | 2005-08-12 23:48:09 -0700 | [diff] [blame] | 86 | [PATCH]. This lets people easily distinguish patches from other |
| 87 | e-mail discussions. |
| 88 | |
| 89 | "git format-patch" command follows the best current practice to |
| 90 | format the body of an e-mail message. At the beginning of the |
| 91 | patch should come your commit message, ending with the |
| 92 | Signed-off-by: lines, and a line that consists of three dashes, |
| 93 | followed by the diffstat information and the patch itself. If |
| 94 | you are forwarding a patch from somebody else, optionally, at |
| 95 | the beginning of the e-mail message just before the commit |
| 96 | message starts, you can put a "From: " line to name that person. |
| 97 | |
| 98 | You often want to add additional explanation about the patch, |
| 99 | other than the commit message itself. Place such "cover letter" |
| 100 | material between the three dash lines and the diffstat. |
| 101 | |
| 102 | Do not attach the patch as a MIME attachment, compressed or not. |
Junio C Hamano | e30b217 | 2007-01-17 01:07:27 -0800 | [diff] [blame] | 103 | Do not let your e-mail client send quoted-printable. Do not let |
| 104 | your e-mail client send format=flowed which would destroy |
| 105 | whitespaces in your patches. Many |
Junio C Hamano | 3140825 | 2005-08-12 23:48:09 -0700 | [diff] [blame] | 106 | popular e-mail applications will not always transmit a MIME |
| 107 | attachment as plain text, making it impossible to comment on |
| 108 | your code. A MIME attachment also takes a bit more time to |
| 109 | process. This does not decrease the likelihood of your |
| 110 | MIME-attached change being accepted, but it makes it more likely |
| 111 | that it will be postponed. |
| 112 | |
| 113 | Exception: If your mailer is mangling patches then someone may ask |
Junio C Hamano | 9847f7e | 2005-08-28 17:54:18 -0700 | [diff] [blame] | 114 | you to re-send them using MIME, that is OK. |
Junio C Hamano | 3140825 | 2005-08-12 23:48:09 -0700 | [diff] [blame] | 115 | |
Junio C Hamano | 9847f7e | 2005-08-28 17:54:18 -0700 | [diff] [blame] | 116 | Do not PGP sign your patch, at least for now. Most likely, your |
| 117 | maintainer or other people on the list would not have your PGP |
| 118 | key and would not bother obtaining it anyway. Your patch is not |
| 119 | judged by who you are; a good patch from an unknown origin has a |
| 120 | far better chance of being accepted than a patch from a known, |
| 121 | respected origin that is done poorly or does incorrect things. |
| 122 | |
| 123 | If you really really really really want to do a PGP signed |
| 124 | patch, format it as "multipart/signed", not a text/plain message |
| 125 | that starts with '-----BEGIN PGP SIGNED MESSAGE-----'. That is |
| 126 | not a text/plain, it's something else. |
| 127 | |
| 128 | Note that your maintainer does not necessarily read everything |
| 129 | on the git mailing list. If your patch is for discussion first, |
| 130 | send it "To:" the mailing list, and optionally "cc:" him. If it |
| 131 | is trivially correct or after the list reached a consensus, send |
| 132 | it "To:" the maintainer and optionally "cc:" the list. |
Junio C Hamano | 3140825 | 2005-08-12 23:48:09 -0700 | [diff] [blame] | 133 | |
Junio C Hamano | 04d2445 | 2006-10-24 01:29:27 -0700 | [diff] [blame] | 134 | Also note that your maintainer does not actively involve himself in |
| 135 | maintaining what are in contrib/ hierarchy. When you send fixes and |
| 136 | enhancements to them, do not forget to "cc: " the person who primarily |
| 137 | worked on that hierarchy in contrib/. |
| 138 | |
Junio C Hamano | 3140825 | 2005-08-12 23:48:09 -0700 | [diff] [blame] | 139 | |
Junio C Hamano | 84ab7b6 | 2006-10-25 14:38:24 -0700 | [diff] [blame] | 140 | (4) Sign your work |
Junio C Hamano | 3140825 | 2005-08-12 23:48:09 -0700 | [diff] [blame] | 141 | |
| 142 | To improve tracking of who did what, we've borrowed the |
| 143 | "sign-off" procedure from the Linux kernel project on patches |
| 144 | that are being emailed around. Although core GIT is a lot |
| 145 | smaller project it is a good discipline to follow it. |
| 146 | |
| 147 | The sign-off is a simple line at the end of the explanation for |
| 148 | the patch, which certifies that you wrote it or otherwise have |
| 149 | the right to pass it on as a open-source patch. The rules are |
| 150 | pretty simple: if you can certify the below: |
| 151 | |
| 152 | Developer's Certificate of Origin 1.1 |
| 153 | |
| 154 | By making a contribution to this project, I certify that: |
| 155 | |
| 156 | (a) The contribution was created in whole or in part by me and I |
| 157 | have the right to submit it under the open source license |
| 158 | indicated in the file; or |
| 159 | |
| 160 | (b) The contribution is based upon previous work that, to the best |
| 161 | of my knowledge, is covered under an appropriate open source |
| 162 | license and I have the right under that license to submit that |
| 163 | work with modifications, whether created in whole or in part |
| 164 | by me, under the same open source license (unless I am |
| 165 | permitted to submit under a different license), as indicated |
| 166 | in the file; or |
| 167 | |
| 168 | (c) The contribution was provided directly to me by some other |
| 169 | person who certified (a), (b) or (c) and I have not modified |
| 170 | it. |
| 171 | |
| 172 | (d) I understand and agree that this project and the contribution |
| 173 | are public and that a record of the contribution (including all |
| 174 | personal information I submit with it, including my sign-off) is |
| 175 | maintained indefinitely and may be redistributed consistent with |
| 176 | this project or the open source license(s) involved. |
| 177 | |
| 178 | then you just add a line saying |
| 179 | |
| 180 | Signed-off-by: Random J Developer <random@developer.example.org> |
| 181 | |
Paolo Ciarrocchi | 6994560 | 2006-11-21 19:55:20 +0100 | [diff] [blame] | 182 | This line can be automatically added by git if you run the git-commit |
| 183 | command with the -s option. |
| 184 | |
Junio C Hamano | 3140825 | 2005-08-12 23:48:09 -0700 | [diff] [blame] | 185 | Some people also put extra tags at the end. They'll just be ignored for |
| 186 | now, but you can do this to mark internal company procedures or just |
| 187 | point out some special detail about the sign-off. |
Junio C Hamano | 9740d28 | 2005-08-26 23:53:07 -0700 | [diff] [blame] | 188 | |
| 189 | |
| 190 | ------------------------------------------------ |
| 191 | MUA specific hints |
| 192 | |
| 193 | Some of patches I receive or pick up from the list share common |
| 194 | patterns of breakage. Please make sure your MUA is set up |
| 195 | properly not to corrupt whitespaces. Here are two common ones |
| 196 | I have seen: |
| 197 | |
| 198 | * Empty context lines that do not have _any_ whitespace. |
| 199 | |
| 200 | * Non empty context lines that have one extra whitespace at the |
| 201 | beginning. |
| 202 | |
Junio C Hamano | 9847f7e | 2005-08-28 17:54:18 -0700 | [diff] [blame] | 203 | One test you could do yourself if your MUA is set up correctly is: |
| 204 | |
| 205 | * Send the patch to yourself, exactly the way you would, except |
| 206 | To: and Cc: lines, which would not contain the list and |
| 207 | maintainer address. |
| 208 | |
| 209 | * Save that patch to a file in UNIX mailbox format. Call it say |
| 210 | a.patch. |
| 211 | |
| 212 | * Try to apply to the tip of the "master" branch from the |
| 213 | git.git public repository: |
| 214 | |
| 215 | $ git fetch http://kernel.org/pub/scm/git/git.git master:test-apply |
| 216 | $ git checkout test-apply |
| 217 | $ git reset --hard |
| 218 | $ git applymbox a.patch |
| 219 | |
| 220 | If it does not apply correctly, there can be various reasons. |
| 221 | |
| 222 | * Your patch itself does not apply cleanly. That is _bad_ but |
| 223 | does not have much to do with your MUA. Please rebase the |
| 224 | patch appropriately. |
| 225 | |
| 226 | * Your MUA corrupted your patch; applymbox would complain that |
| 227 | the patch does not apply. Look at .dotest/ subdirectory and |
| 228 | see what 'patch' file contains and check for the common |
| 229 | corruption patterns mentioned above. |
| 230 | |
| 231 | * While you are at it, check what are in 'info' and |
| 232 | 'final-commit' files as well. If what is in 'final-commit' is |
| 233 | not exactly what you would want to see in the commit log |
| 234 | message, it is very likely that your maintainer would end up |
| 235 | hand editing the log message when he applies your patch. |
| 236 | Things like "Hi, this is my first patch.\n", if you really |
| 237 | want to put in the patch e-mail, should come after the |
| 238 | three-dash line that signals the end of the commit message. |
| 239 | |
Junio C Hamano | 9740d28 | 2005-08-26 23:53:07 -0700 | [diff] [blame] | 240 | |
| 241 | Pine |
| 242 | ---- |
| 243 | |
| 244 | (Johannes Schindelin) |
| 245 | |
| 246 | I don't know how many people still use pine, but for those poor |
| 247 | souls it may be good to mention that the quell-flowed-text is |
| 248 | needed for recent versions. |
| 249 | |
| 250 | ... the "no-strip-whitespace-before-send" option, too. AFAIK it |
| 251 | was introduced in 4.60. |
| 252 | |
| 253 | (Linus Torvalds) |
| 254 | |
| 255 | And 4.58 needs at least this. |
| 256 | |
| 257 | --- |
| 258 | diff-tree 8326dd8350be64ac7fc805f6563a1d61ad10d32c (from e886a61f76edf5410573e92e38ce22974f9c40f1) |
| 259 | Author: Linus Torvalds <torvalds@g5.osdl.org> |
| 260 | Date: Mon Aug 15 17:23:51 2005 -0700 |
| 261 | |
| 262 | Fix pine whitespace-corruption bug |
| 263 | |
| 264 | There's no excuse for unconditionally removing whitespace from |
| 265 | the pico buffers on close. |
| 266 | |
| 267 | diff --git a/pico/pico.c b/pico/pico.c |
| 268 | --- a/pico/pico.c |
| 269 | +++ b/pico/pico.c |
| 270 | @@ -219,7 +219,9 @@ PICO *pm; |
| 271 | switch(pico_all_done){ /* prepare for/handle final events */ |
| 272 | case COMP_EXIT : /* already confirmed */ |
| 273 | packheader(); |
| 274 | +#if 0 |
| 275 | stripwhitespace(); |
| 276 | +#endif |
| 277 | c |= COMP_EXIT; |
| 278 | break; |
| 279 | |
| 280 | |
Junio C Hamano | 1eb446f | 2005-08-31 11:48:41 -0700 | [diff] [blame] | 281 | (Daniel Barkalow) |
| 282 | |
| 283 | > A patch to SubmittingPatches, MUA specific help section for |
| 284 | > users of Pine 4.63 would be very much appreciated. |
| 285 | |
| 286 | Ah, it looks like a recent version changed the default behavior to do the |
| 287 | right thing, and inverted the sense of the configuration option. (Either |
| 288 | that or Gentoo did it.) So you need to set the |
| 289 | "no-strip-whitespace-before-send" option, unless the option you have is |
| 290 | "strip-whitespace-before-send", in which case you should avoid checking |
| 291 | it. |
| 292 | |
Junio C Hamano | 9740d28 | 2005-08-26 23:53:07 -0700 | [diff] [blame] | 293 | |
| 294 | Thunderbird |
| 295 | ----------- |
| 296 | |
| 297 | (A Large Angry SCM) |
| 298 | |
| 299 | Here are some hints on how to successfully submit patches inline using |
A Large Angry SCM | cf6de18 | 2005-08-29 22:34:07 -0400 | [diff] [blame] | 300 | Thunderbird. |
Junio C Hamano | 9740d28 | 2005-08-26 23:53:07 -0700 | [diff] [blame] | 301 | |
| 302 | This recipe appears to work with the current [*1*] Thunderbird from Suse. |
| 303 | |
| 304 | The following Thunderbird extensions are needed: |
| 305 | AboutConfig 0.5 |
| 306 | http://aboutconfig.mozdev.org/ |
Lukas Sandström | ff62b7f | 2006-05-18 14:23:59 +0200 | [diff] [blame] | 307 | External Editor 0.7.2 |
| 308 | http://globs.org/articles.php?lng=en&pg=8 |
Junio C Hamano | 9740d28 | 2005-08-26 23:53:07 -0700 | [diff] [blame] | 309 | |
| 310 | 1) Prepare the patch as a text file using your method of choice. |
| 311 | |
| 312 | 2) Before opening a compose window, use Edit->Account Settings to |
| 313 | uncheck the "Compose messages in HTML format" setting in the |
| 314 | "Composition & Addressing" panel of the account to be used to send the |
| 315 | patch. [*2*] |
| 316 | |
| 317 | 3) In the main Thunderbird window, _before_ you open the compose window |
| 318 | for the patch, use Tools->about:config to set the following to the |
| 319 | indicated values: |
| 320 | mailnews.send_plaintext_flowed => false |
A Large Angry SCM | cf6de18 | 2005-08-29 22:34:07 -0400 | [diff] [blame] | 321 | mailnews.wraplength => 0 |
Junio C Hamano | 9740d28 | 2005-08-26 23:53:07 -0700 | [diff] [blame] | 322 | |
| 323 | 4) Open a compose window and click the external editor icon. |
| 324 | |
| 325 | 5) In the external editor window, read in the patch file and exit the |
| 326 | editor normally. |
| 327 | |
| 328 | 6) Back in the compose window: Add whatever other text you wish to the |
| 329 | message, complete the addressing and subject fields, and press send. |
| 330 | |
| 331 | 7) Optionally, undo the about:config/account settings changes made in |
| 332 | steps 2 & 3. |
| 333 | |
| 334 | |
| 335 | [Footnotes] |
| 336 | *1* Version 1.0 (20041207) from the MozillaThunderbird-1.0-5 rpm of Suse |
| 337 | 9.3 professional updates. |
| 338 | |
| 339 | *2* It may be possible to do this with about:config and the following |
| 340 | settings but I haven't tried, yet. |
| 341 | mail.html_compose => false |
| 342 | mail.identity.default.compose_html => false |
| 343 | mail.identity.id?.compose_html => false |
| 344 | |
Junio C Hamano | e30b217 | 2007-01-17 01:07:27 -0800 | [diff] [blame] | 345 | |
Junio C Hamano | e30b217 | 2007-01-17 01:07:27 -0800 | [diff] [blame] | 346 | Gnus |
| 347 | ---- |
| 348 | |
| 349 | '|' in the *Summary* buffer can be used to pipe the current |
| 350 | message to an external program, and this is a handy way to drive |
| 351 | "git am". However, if the message is MIME encoded, what is |
| 352 | piped into the program is the representation you see in your |
| 353 | *Article* buffer after unwrapping MIME. This is often not what |
| 354 | you would want for two reasons. It tends to screw up non ASCII |
| 355 | characters (most notably in people's names), and also |
| 356 | whitespaces (fatal in patches). Running 'C-u g' to display the |
| 357 | message in raw form before using '|' to run the pipe can work |
| 358 | this problem around. |
| 359 | |
Michael | 451fd65 | 2007-02-05 14:27:32 +0100 | [diff] [blame] | 360 | |
| 361 | KMail |
| 362 | ----- |
| 363 | |
| 364 | This should help you to submit patches inline using KMail. |
| 365 | |
| 366 | 1) Prepare the patch as a text file. |
| 367 | |
| 368 | 2) Click on New Mail. |
| 369 | |
| 370 | 3) Go under "Options" in the Composer window and be sure that |
| 371 | "Word wrap" is not set. |
| 372 | |
| 373 | 4) Use Message -> Insert file... and insert the patch. |
| 374 | |
| 375 | 5) Back in the compose window: add whatever other text you wish to the |
| 376 | message, complete the addressing and subject fields, and press send. |