diff options
Diffstat (limited to 'Documentation/SubmittingPatches')
| -rw-r--r-- | Documentation/SubmittingPatches | 230 |
1 files changed, 131 insertions, 99 deletions
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 0dbf2c9843..98fc4cc1d0 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -1,65 +1,5 @@ -Checklist (and a short version for the impatient): - - Commits: - - - make commits of logical units - - check for unnecessary whitespace with "git diff --check" - before committing - - do not check in commented out code or unneeded files - - the first line of the commit message should be a short - description (50 characters is the soft limit, see DISCUSSION - in git-commit(1)), and should skip the full stop - - the body should provide a meaningful commit message, which: - . explains the problem the change tries to solve, iow, what - is wrong with the current code without the change. - . justifies the way the change solves the problem, iow, why - the result with the change is better. - . alternate solutions considered but discarded, if any. - - describe changes in imperative mood, e.g. "make xyzzy do frotz" - instead of "[This patch] makes xyzzy do frotz" or "[I] changed - xyzzy to do frotz", as if you are giving orders to the codebase - to change its behaviour. - - try to make sure your explanation can be understood without - external resources. Instead of giving a URL to a mailing list - archive, summarize the relevant points of the discussion. - - add a "Signed-off-by: Your Name <you@example.com>" line to the - commit message (or just use the option "-s" when committing) - to confirm that you agree to the Developer's Certificate of Origin - - make sure that you have tests for the bug you are fixing - - make sure that the test suite passes after your commit - - Patch: - - - use "git format-patch -M" to create the patch - - do not PGP sign your patch - - do not attach your patch, but read in the mail - body, unless you cannot teach your mailer to - leave the formatting of the patch alone. - - be careful doing cut & paste into your mailer, not to - corrupt whitespaces. - - provide additional information (which is unsuitable for - the commit message) between the "---" and the diffstat - - if you change, add, or remove a command line option or - make some other user interface change, the associated - documentation should be updated as well. - - if your name is not writable in ASCII, make sure that - you send off a message in the correct encoding. - - send the patch to the list (git@vger.kernel.org) and the - maintainer (gitster@pobox.com) if (and only if) the patch - is ready for inclusion. If you use git-send-email(1), - please test it first by sending email to yourself. - - see below for instructions specific to your mailer - -Long version: - -I started reading over the SubmittingPatches document for Linux -kernel, primarily because I wanted to have a document similar to -it for the core GIT to make sure people understand what they are -doing when they write "Signed-off-by" line. - -But the patch submission requirements are a lot more relaxed -here on the technical/contents front, because the core GIT is -thousand times smaller ;-). So here is only the relevant bits. +Here are some guidelines for people who want to contribute their code +to this software. (0) Decide what to base your work on. @@ -86,6 +26,10 @@ change is relevant to. wait until some of the dependent topics graduate to 'master', and rebase your work. + - Some parts of the system have dedicated maintainers with their own + repositories (see the section "Subsystems" below). Changes to + these parts should be based on their trees. + To find the tip of a topic branch, run "git log --first-parent master..pu" and look for the merge commit. The second parent of this commit is the tip of the topic branch. @@ -113,49 +57,107 @@ change, the approach taken by the change, and if relevant how this differs substantially from the prior version, are all good things to have. -Oh, another thing. I am picky about whitespaces. Make sure your +Make sure that you have tests for the bug you are fixing. See +t/README for guidance. + +When adding a new feature, make sure that you have new tests to show +the feature triggers the new behaviour when it should, and to show the +feature does not trigger when it shouldn't. Also make sure that the +test suite passes after your commit. Do not forget to update the +documentation to describe the updated behaviour. + +Speaking of the documentation, it is currently a liberal mixture of US +and UK English norms for spelling and grammar, which is somewhat +unfortunate. A huge patch that touches the files all over the place +only to correct the inconsistency is not welcome, though. Potential +clashes with other changes that can result from such a patch are not +worth it. We prefer to gradually reconcile the inconsistencies in +favor of US English, with small and easily digestible patches, as a +side effect of doing some other real work in the vicinity (e.g. +rewriting a paragraph for clarity, while turning en_UK spelling to +en_US). Obvious typographical fixes are much more welcomed ("teh -> +"the"), preferably submitted as independent patches separate from +other documentation changes. + +Oh, another thing. We are picky about whitespaces. Make sure your changes do not trigger errors with the sample pre-commit hook shipped in templates/hooks--pre-commit. To help ensure this does not happen, run git diff --check on your changes before you commit. -(1a) Try to be nice to older C compilers +(2) Describe your changes well. + +The first line of the commit message should be a short description (50 +characters is the soft limit, see DISCUSSION in git-commit(1)), and +should skip the full stop. It is also conventional in most cases to +prefix the first line with "area: " where the area is a filename or +identifier for the general area of the code being modified, e.g. + + . archive: ustar header checksum is computed unsigned + . git-cherry-pick.txt: clarify the use of revision range notation + +If in doubt which identifier to use, run "git log --no-merges" on the +files you are modifying to see the current conventions. + +The body should provide a meaningful commit message, which: + + . explains the problem the change tries to solve, iow, what is wrong + with the current code without the change. -We try to support a wide range of C compilers to compile -git with. That means that you should not use C99 initializers, even -if a lot of compilers grok it. + . justifies the way the change solves the problem, iow, why the + result with the change is better. -Also, variables have to be declared at the beginning of the block -(you can check this with gcc, using the -Wdeclaration-after-statement -option). + . alternate solutions considered but discarded, if any. -Another thing: NULL pointers shall be written as NULL, not as 0. +Describe your changes in imperative mood, e.g. "make xyzzy do frotz" +instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy +to do frotz", as if you are giving orders to the codebase to change +its behaviour. Try to make sure your explanation can be understood +without external resources. Instead of giving a URL to a mailing list +archive, summarize the relevant points of the discussion. -(2) Generate your patch using git tools out of your commits. +(3) Generate your patch using Git tools out of your commits. -git based diff tools generate unidiff which is the preferred format. +Git based diff tools generate unidiff which is the preferred format. You do not have to be afraid to use -M option to "git diff" or "git format-patch", if your patch involves file renames. The receiving end can handle them just fine. -Please make sure your patch does not include any extra files -which do not belong in a patch submission. Make sure to review +Please make sure your patch does not add commented out debugging code, +or include any extra files which do not relate to what your patch +is trying to achieve. Make sure to review your patch after generating it, to ensure accuracy. Before sending out, please make sure it cleanly applies to the "master" branch head. If you are preparing a work based on "next" branch, that is fine, but please mark it as such. -(3) Sending your patches. +(4) Sending your patches. -People on the git mailing list need to be able to read and +Learn to use format-patch and send-email if possible. These commands +are optimized for the workflow of sending patches, avoiding many ways +your existing e-mail client that is optimized for "multipart/*" mime +type e-mails to corrupt and render your patches unusable. + +People on the Git mailing list need to be able to read and comment on the changes you are submitting. It is important for a developer to be able to "quote" your changes, using standard e-mail tools, so that they may comment on specific portions of -your code. For this reason, all patches should be submitted -"inline". WARNING: Be wary of your MUAs word-wrap +your code. For this reason, each patch should be submitted +"inline" in a separate message. + +Multiple related patches should be grouped into their own e-mail +thread to help readers find all parts of the series. To that end, +send them as replies to either an additional "cover letter" message +(see below), the first patch, or the respective preceding patch. + +If your log message (including your name on the +Signed-off-by line) is not writable in ASCII, make sure that +you send off a message in the correct encoding. + +WARNING: Be wary of your MUAs word-wrap corrupting your patch. Do not cut-n-paste your patch; you can lose tabs that way if you are not careful. @@ -179,7 +181,11 @@ message starts, you can put a "From: " line to name that person. You often want to add additional explanation about the patch, other than the commit message itself. Place such "cover letter" -material between the three dash lines and the diffstat. +material between the three-dash line and the diffstat. For +patches requiring multiple iterations of review and discussion, +an explanation of changes between each iteration can be kept in +Git-notes and inserted automatically following the three-dash +line via `git format-patch --notes`. Do not attach the patch as a MIME attachment, compressed or not. Do not let your e-mail client send quoted-printable. Do not let @@ -207,23 +213,29 @@ patch, format it as "multipart/signed", not a text/plain message that starts with '-----BEGIN PGP SIGNED MESSAGE-----'. That is not a text/plain, it's something else. -Unless your patch is a very trivial and an obviously correct one, -first send it with "To:" set to the mailing list, with "cc:" listing +Send your patch with "To:" set to the mailing list, with "cc:" listing people who are involved in the area you are touching (the output from "git blame $path" and "git shortlog --no-merges $path" would help to -identify them), to solicit comments and reviews. After the list -reached a consensus that it is a good idea to apply the patch, re-send -it with "To:" set to the maintainer and optionally "cc:" the list for -inclusion. Do not forget to add trailers such as "Acked-by:", -"Reviewed-by:" and "Tested-by:" after your "Signed-off-by:" line as -necessary. +identify them), to solicit comments and reviews. + +After the list reached a consensus that it is a good idea to apply the +patch, re-send it with "To:" set to the maintainer [*1*] and "cc:" the +list [*2*] for inclusion. + +Do not forget to add trailers such as "Acked-by:", "Reviewed-by:" and +"Tested-by:" lines as necessary to credit people who helped your +patch. + [Addresses] + *1* The current maintainer: gitster@pobox.com + *2* The mailing list: git@vger.kernel.org -(4) Sign your work + +(5) Sign your work To improve tracking of who did what, we've borrowed the "sign-off" procedure from the Linux kernel project on patches -that are being emailed around. Although core GIT is a lot +that are being emailed around. Although core Git is a lot smaller project it is a good discipline to follow it. The sign-off is a simple line at the end of the explanation for @@ -251,17 +263,17 @@ pretty simple: if you can certify the below: person who certified (a), (b) or (c) and I have not modified it. - (d) I understand and agree that this project and the contribution - are public and that a record of the contribution (including all - personal information I submit with it, including my sign-off) is - maintained indefinitely and may be redistributed consistent with - this project or the open source license(s) involved. + (d) I understand and agree that this project and the contribution + are public and that a record of the contribution (including all + personal information I submit with it, including my sign-off) is + maintained indefinitely and may be redistributed consistent with + this project or the open source license(s) involved. then you just add a line saying - Signed-off-by: Random J Developer <random@developer.example.org> + Signed-off-by: Random J Developer <random@developer.example.org> -This line can be automatically added by git if you run the git-commit +This line can be automatically added by Git if you run the git-commit command with the -s option. Notice that you can place your own Signed-off-by: line when @@ -290,6 +302,26 @@ You can also create your own tag or use one that's in common usage such as "Thanks-to:", "Based-on-patch-by:", or "Mentored-by:". ------------------------------------------------ +Subsystems with dedicated maintainers + +Some parts of the system have dedicated maintainers with their own +repositories. + + - git-gui/ comes from git-gui project, maintained by Pat Thoyts: + + git://repo.or.cz/git-gui.git + + - gitk-git/ comes from Paul Mackerras's gitk project: + + git://ozlabs.org/~paulus/gitk + + - po/ comes from the localization coordinator, Jiang Xin: + + https://github.com/git-l10n/git-po/ + +Patches to these parts should be based on their trees. + +------------------------------------------------ An ideal patch flow Here is an ideal patch flow for this project the current maintainer @@ -314,7 +346,7 @@ suggests to the contributors: spend their time to improve your patch. Go back to step (2). (4) The list forms consensus that the last round of your patch is - good. Send it to the list and cc the maintainer. + good. Send it to the maintainer and cc the list. (5) A topic branch is created with the patch and is merged to 'next', and cooked further and eventually graduates to 'master'. @@ -334,7 +366,7 @@ Know the status of your patch after submission tell you if your patch is merged in pu if you rebase on top of master). -* Read the git mailing list, the maintainer regularly posts messages +* Read the Git mailing list, the maintainer regularly posts messages entitled "What's cooking in git.git" and "What's in git.git" giving the status of various proposed changes. |