diff options
Diffstat (limited to 'Documentation/SubmittingPatches')
| -rw-r--r-- | Documentation/SubmittingPatches | 275 |
1 files changed, 197 insertions, 78 deletions
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index c647c7e1b4..d8a8caa791 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. @@ -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]), @@ -397,17 +465,57 @@ 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 (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 a developer to be able to "quote" your changes, using standard @@ -415,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 @@ -498,42 +608,93 @@ 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: @@ -548,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 to 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" giving - the status of various proposed changes. == GitHub CI[[GHCI]] |