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