diff options
Diffstat (limited to 'Documentation/SubmittingPatches')
| -rw-r--r-- | Documentation/SubmittingPatches | 334 |
1 files changed, 239 insertions, 95 deletions
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 973d7a81d4..db17bc7fe2 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -7,6 +7,73 @@ Here are some guidelines for contributing back to this project. There is also a link:MyFirstContribution.html[step-by-step tutorial] available which covers many of these same guidelines. +[[patch-flow]] +=== A typical life cycle of a patch series + +To help us understand the reason behind various guidelines given later +in the document, first let's understand how the life cycle of a +typical patch series for this project goes. + +. You come up with an itch. You code it up. You do not need any + pre-authorization from the project to do so. ++ +Your patches will be reviewed by other contributors on the mailing +list, and the reviews will be done to assess the merit of various +things, like the general idea behind your patch (including "is it +solving a problem worth solving in the first place?"), the reason +behind the design of the solution, and the actual implementation. +The guidelines given here are there to help your patches by making +them easier to understand by the reviewers. + +. You send the patches to the list and cc people who may need to know + about the change. Your goal is *not* necessarily to convince others + that what you are building is good. Your goal is to get help in + coming up with a solution for the "itch" that is better than what + you can build alone. ++ +The people who may need to know are the ones who worked on the code +you are touching. These people happen to be the ones who are +most likely to be knowledgeable enough to help you, but +they have no obligation to help you (i.e. you ask them for help, +you don't demand). +git log -p {litdd} _$area_you_are_modifying_+ would +help you find out who they are. + +. You get comments and suggestions for improvements. You may even get + them in an "on top of your change" patch form. You are expected to + respond to them with "Reply-All" on the mailing list, while taking + them into account while preparing an updated set of patches. + +. Polish, refine, and re-send your patches to the list and to the people + who spent their time to improve your patch. Go back to step (2). + +. While the above iterations improve your patches, the maintainer may + pick the patches up from the list and queue them to the `seen` + branch, in order to make it easier for people to play with it + without having to pick up and apply the patches to their trees + themselves. Being in `seen` has no other meaning. Specifically, it + does not mean the patch was "accepted" in any way. + +. When the discussion reaches a consensus that the latest iteration of + the patches are in good enough shape, the maintainer includes the + topic in the "What's cooking" report that are sent out a few times a + week to the mailing list, marked as "Will merge to 'next'." This + decision is primarily made by the maintainer with help from those + who participated in the review discussion. + +. After the patches are merged to the 'next' branch, the discussion + can still continue to further improve them by adding more patches on + top, but by the time a topic gets merged to 'next', it is expected + that everybody agrees that the scope and the basic direction of the + topic are appropriate, so such an incremental updates are limited to + small corrections and polishing. After a topic cooks for some time + (like 7 calendar days) in 'next' without needing further tweaks on + top, it gets merged to the 'master' branch and wait to become part + of the next major release. + +In the following sections, many techniques and conventions are listed +to help your patches get reviewed effectively in such a life cycle. + + [[choose-starting-point]] === Choose a starting point. @@ -87,7 +154,7 @@ maintainer. Under truly exceptional circumstances where you absolutely must depend on a select few topic branches that are already in `next` but not in `master`, you may want to create your own custom base-branch by forking -`master` and merging the required topic branches to it. You could then +`master` and merging the required topic branches into it. You could then work on top of this base-branch. But keep in mind that this base-branch would only be known privately to you. So when you are ready to send your patches to the list, be sure to communicate how you created it in @@ -192,8 +259,9 @@ reasons: which case, they can explain why they extend your code to cover files, too). -The goal of your log message is to convey the _why_ behind your -change to help future developers. +The goal of your log message is to convey the _why_ behind your change +to help future developers. The reviewers will also make sure that +your proposed log message will serve this purpose well. The first line of the commit message should be a short description (50 characters is the soft limit, see DISCUSSION in linkgit:git-commit[1]), @@ -266,7 +334,7 @@ date)", like this: noticed that ... .... -The "Copy commit summary" command of gitk can be used to obtain this +The "Copy commit reference" command of gitk can be used to obtain this format (with the subject enclosed in a pair of double-quotes), or this invocation of `git show`: @@ -344,20 +412,32 @@ Also notice that a real name is used in the `Signed-off-by` trailer. Please don't hide your real name. [[commit-trailers]] -If you like, you can put extra tags at the end: +If you like, you can put extra trailers at the end: . `Reported-by:` is used to credit someone who found the bug that the patch attempts to fix. . `Acked-by:` says that the person who is more familiar with the area the patch attempts to modify liked the patch. -. `Reviewed-by:`, unlike the other tags, can only be offered by the +. `Reviewed-by:`, unlike the other trailers, can only be offered by the reviewers themselves when they are completely satisfied with the patch after a detailed analysis. . `Tested-by:` is used to indicate that the person applied the patch and found it to have the desired effect. - -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:". +. `Co-authored-by:` is used to indicate that people exchanged drafts + of a patch before submitting it. +. `Helped-by:` is used to credit someone who suggested ideas for + changes without providing the precise changes in patch form. +. `Mentored-by:` is used to credit someone with helping develop a + patch as part of a mentorship program (e.g., GSoC or Outreachy). +. `Suggested-by:` is used to credit someone with suggesting the idea + for a patch. + +While you can also create your own trailer if the situation warrants it, we +encourage you to instead use one of the common trailers in this project +highlighted above. + +Only capitalize the very first letter of the trailer, i.e. favor +"Signed-off-by" over "Signed-Off-By" and "Acked-by:" over "Acked-By". [[git-tools]] === Generate your patch using Git tools out of your commits. @@ -385,16 +465,56 @@ letter. [[send-patches]] === Sending your patches. +==== Choosing your reviewers + :security-ml: footnoteref:[security-ml,The Git Security mailing list: git-security@googlegroups.com] -Before sending any patches, please note that patches that may be +NOTE: Patches that may be security relevant should be submitted privately to the Git Security mailing list{security-ml}, instead of the public mailing list. -Learn to use format-patch and send-email if possible. These commands +:contrib-scripts: footnoteref:[contrib-scripts,Scripts under `contrib/` are + +not part of the core `git` binary and must be called directly. Clone the Git + +codebase and run `perl contrib/contacts/git-contacts`.] + +Send your patch with "To:" set to the mailing list, with "cc:" listing +people who are involved in the area you are touching (the `git-contacts` +script in `contrib/contacts/`{contrib-scripts} can help to +identify them), to solicit comments and reviews. Also, when you made +trial merges of your topic to `next` and `seen`, you may have noticed +work by others conflicting with your changes. There is a good possibility +that these people may know the area you are touching well. + +If you are using `send-email`, you can feed it the output of `git-contacts` like +this: + +.... + git send-email --cc-cmd='perl contrib/contacts/git-contacts' feature/*.patch +.... + +:current-maintainer: footnote:[The current maintainer: gitster@pobox.com] +:git-ml: footnote:[The mailing list: git@vger.kernel.org] + +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{current-maintainer} +and "cc:" the list{git-ml} for inclusion. This is especially relevant +when the maintainer did not heavily participate in the discussion and +instead left the review to trusted others. + +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, and "cc:" them when sending such a final version for inclusion. + +==== `format-patch` and `send-email` + +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. +your existing e-mail client (often optimized for "multipart/*" MIME +type e-mails) might render your patches unusable. + +NOTE: Here we outline the procedure using `format-patch` and +`send-email`, but you can instead use GitGitGadget to send in your +patches (see link:MyFirstContribution.html[MyFirstContribution]). People on the Git mailing list need to be able to read and comment on the changes you are submitting. It is important for @@ -403,10 +523,12 @@ e-mail tools, so that they may comment on specific portions of 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. +All subsequent versions of a patch series and other 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. +Here is a link:MyFirstContribution.html#v2-git-send-email[step-by-step guide] on +how to submit updated versions of a patch series. If your log message (including your name on the `Signed-off-by` trailer) is not writable in ASCII, make sure that @@ -447,6 +569,18 @@ 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`. +[[the-topic-summary]] +*This is EXPERIMENTAL*. + +When sending a topic, you can propose a one-paragraph summary that +should appear in the "What's cooking" report when it is picked up to +explain the topic. If you choose to do so, please write a 2-5 line +paragraph that will fit well in our release notes (see many bulleted +entries in the Documentation/RelNotes/* files for examples), and make +it the first paragraph of the cover letter. For a single-patch +series, use the space between the three-dash line and the diffstat, as +described earlier. + [[attachment]] 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 @@ -474,49 +608,100 @@ 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. -:security-ml-ref: footnoteref:[security-ml] +=== Handling Conflicts and Iterating Patches -As mentioned at the beginning of the section, patches that may be -security relevant should not be submitted to the public mailing list -mentioned below, but should instead be sent privately to the Git -Security mailing list{security-ml-ref}. +When revising changes made to your patches, it's important to +acknowledge the possibility of conflicts with other ongoing topics. To +navigate these potential conflicts effectively, follow the recommended +steps outlined below: -Send your patch with "To:" set to the mailing list, with "cc:" listing -people who are involved in the area you are touching (the `git -contacts` command in `contrib/contacts/` can help to -identify them), to solicit comments and reviews. Also, when you made -trial merges of your topic to `next` and `seen`, you may have noticed -work by others conflicting with your changes. There is a good possibility -that these people may know the area you are touching well. +. Build on a suitable base branch, see the <<choose-starting-point, section above>>, +and format-patch the series. If you are doing "rebase -i" in-place to +update from the previous round, this will reuse the previous base so +(2) and (3) may become trivial. -:current-maintainer: footnote:[The current maintainer: gitster@pobox.com] -:git-ml: footnote:[The mailing list: git@vger.kernel.org] +. Find the base of where the last round was queued ++ + $ mine='kn/ref-transaction-symref' + $ git checkout "origin/seen^{/^Merge branch '$mine'}...master" -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{current-maintainer} -and "cc:" the list{git-ml} for inclusion. This is especially relevant -when the maintainer did not heavily participate in the discussion and -instead left the review to trusted others. +. Apply your format-patch result. There are two cases +.. Things apply cleanly and tests fine. Go to (4). +.. Things apply cleanly but does not build or test fails, or things do +not apply cleanly. ++ +In the latter case, you have textual or semantic conflicts coming from +the difference between the old base and the base you used to build in +(1). Identify what caused the breakages (e.g., a topic or two may have +merged since the base used by (2) until the base used by (1)). ++ +Check out the latest 'origin/master' (which may be newer than the base +used by (2)), "merge --no-ff" the topics you newly depend on in there, +and use the result of the merge(s) as the base, rebuild the series and +test again. Run format-patch from the last such merges to the tip of +your topic. If you did ++ + $ git checkout origin/master + $ git merge --no-ff --into-name kn/ref-transaction-symref fo/obar + $ git merge --no-ff --into-name kn/ref-transaction-symref ba/zqux + ... rebuild the topic ... ++ +Then you'd just format your topic above these "preparing the ground" +merges, e.g. ++ + $ git format-patch "HEAD^{/^Merge branch 'ba/zqux'}"..HEAD ++ +Do not forget to write in the cover letter you did this, including the +topics you have in your base on top of 'master'. Then go to (4). -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, and "cc:" them when sending such a final version for inclusion. +. Make a trial merge of your topic into 'next' and 'seen', e.g. ++ + $ git checkout --detach 'origin/seen' + $ git revert -m 1 <the merge of the previous iteration into seen> + $ git merge kn/ref-transaction-symref ++ +The "revert" is needed if the previous iteration of your topic is +already in 'seen' (like in this case). You could choose to rebuild +master..origin/seen from scratch while excluding your previous +iteration, which may emulate what happens on the maintainers end more +closely. ++ +This trial merge may conflict. It is primarily to see what conflicts +_other_ topics may have with your topic. In other words, you do not +have to depend on it to make your topic work on 'master'. It may +become the job of the other topic owners to resolve conflicts if your +topic goes to 'next' before theirs. ++ +Make a note on what conflict you saw in the cover letter. You do not +necessarily have to resolve them, but it would be a good opportunity to +learn what others are doing in related areas. ++ + $ git checkout --detach 'origin/next' + $ git merge kn/ref-transaction-symref ++ +This is to see what conflicts your topic has with other topics that are +already cooking. This should not conflict if (3)-2 prepared a base on +top of updated master plus dependent topics taken from 'next'. Unless +the context is severe (one way to tell is try the same trial merge with +your old iteration, which may conflict in a similar way), expect that it +will be handled on maintainers end (if it gets unmanageable, I'll ask to +rebase when I receive your patches). == 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 Pratyush Yadav: +- `git-gui/` comes from git-gui project, maintained by Johannes Sixt: - https://github.com/prati0100/git-gui.git + https://github.com/j6t/git-gui - `gitk-git/` comes from Paul Mackerras's gitk project: git://git.ozlabs.org/~paulus/gitk - Those who are interested in improve gitk can volunteer to help Paul - in maintaining it cf. <YntxL/fTplFm8lr6@cleo>. + Those who are interested in improving gitk can volunteer to help Paul + maintain it, cf. <YntxL/fTplFm8lr6@cleo>. - `po/` comes from the localization coordinator, Jiang Xin: @@ -524,54 +709,12 @@ repositories. Patches to these parts should be based on their trees. -[[patch-flow]] -== An ideal patch flow - -Here is an ideal patch flow for this project the current maintainer -suggests to the contributors: - -. You come up with an itch. You code it up. - -. Send it to the list and cc people who may need to know about - the change. -+ -The people who may need to know are the ones whose code you -are butchering. These people happen to be the ones who are -most likely to be knowledgeable enough to help you, but -they have no obligation to help you (i.e. you ask for help, -don't demand). +git log -p {litdd} _$area_you_are_modifying_+ would -help you find out who they are. - -. You get comments and suggestions for improvements. You may - even get them in an "on top of your change" patch form. - -. Polish, refine, and re-send to the list and the people who - spend their time to improve your patch. Go back to step (2). - -. The list forms consensus that the last round of your patch is - good. Send it to the maintainer and cc the list. - -. A topic branch is created with the patch and is merged to `next`, - and cooked further and eventually graduates to `master`. - -In any time between the (2)-(3) cycle, the maintainer may pick it up -from the list and queue it to `seen`, in order to make it easier for -people play with it without having to pick up and apply the patch to -their trees themselves. - -[[patch-status]] -== Know the status of your patch after submission +- The "Git documentation translations" project, led by Jean-Noël + Avila, translates our documentation pages. Their work products are + maintained separately from this project, not as part of our tree: -* You can use Git itself to find out when your patch is merged in - master. `git pull --rebase` will automatically skip already-applied - patches, and will let you know. This works only if you rebase on top - of the branch in which your patch has been merged (i.e. it will not - tell you if your patch is merged in `seen` if you rebase on top of - master). + https://github.com/jnavila/git-manpages-l10n/ -* 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. == GitHub CI[[GHCI]] @@ -590,11 +733,12 @@ After the initial setup, CI will run whenever you push new changes to your fork of Git on GitHub. You can monitor the test state of all your branches here: `https://github.com/<Your GitHub handle>/git/actions/workflows/main.yml` -If a branch did not pass all test cases then it is marked with a red -cross. In that case you can click on the failing job and navigate to -"ci/run-build-and-tests.sh" and/or "ci/print-test-failures.sh". You -can also download "Artifacts" which are tarred (or zipped) archives -with test data relevant for debugging. +If a branch does not pass all test cases then it will be marked with a +red +x+, instead of a green check. In that case, you can click on the +failing job and navigate to "ci/run-build-and-tests.sh" and/or +"ci/print-test-failures.sh". You can also download "Artifacts" which +are zip archives containing tarred (or zipped) archives with test data +relevant for debugging. Then fix the problem and push your fix to your GitHub fork. This will trigger a new CI build to ensure all tests pass. @@ -686,7 +830,7 @@ message to an external program, and this is a handy way to drive `git am`. However, if the message is MIME encoded, what is piped into the program is the representation you see in your `*Article*` buffer after unwrapping MIME. This is often not what -you would want for two reasons. It tends to screw up non ASCII +you would want for two reasons. It tends to screw up non-ASCII characters (most notably in people's names), and also whitespaces (fatal in patches). Running "C-u g" to display the message in raw form before using "|" to run the pipe can work |