diff options
Diffstat (limited to 'Documentation/SubmittingPatches')
-rw-r--r-- | Documentation/SubmittingPatches | 89 |
1 files changed, 75 insertions, 14 deletions
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 11e03056f2..a6121d1d42 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -19,8 +19,10 @@ change is relevant to. base your work on the tip of the topic. * A new feature should be based on `master` in general. If the new - feature depends on a topic that is in `seen`, but not in `master`, - base your work on the tip of that topic. + feature depends on other topics that are in `next`, but not in + `master`, fork a branch from the tip of `master`, merge these topics + to the branch, and work on that branch. You can remind yourself of + how you prepared the base with `git log --first-parent master..`. * Corrections and enhancements to a topic not yet in `master` should be based on the tip of that topic. If the topic has not been merged @@ -28,10 +30,10 @@ change is relevant to. into the series. * In the exceptional case that a new feature depends on several topics - not in `master`, start working on `next` or `seen` privately and send - out patches for discussion. Before the final merge, you may have to - wait until some of the dependent topics graduate to `master`, and - rebase your work. + not in `master`, start working on `next` or `seen` privately and + send out patches only for discussion. Once your new feature starts + to stabilize, you would have to rebase it (see the "depends on other + topics" above). * Some parts of the system have dedicated maintainers with their own repositories (see the section "Subsystems" below). Changes to @@ -71,8 +73,13 @@ Make sure that you have tests for the bug you are fixing. See [[tests]] When adding a new feature, make sure that you have new tests to show the feature triggers the new behavior when it should, and to show the -feature does not trigger when it shouldn't. After any code change, make -sure that the entire test suite passes. +feature does not trigger when it shouldn't. After any code change, +make sure that the entire test suite passes. When fixing a bug, make +sure you have new tests that break if somebody else breaks what you +fixed by accident to avoid regression. Also, try merging your work to +'next' and 'seen' and make sure the tests still pass; topics by others +that are still in flight may have unexpected interactions with what +you are trying to do in your topic. Pushing to a fork of https://github.com/git/git will use their CI integration to test your changes on Linux, Mac and Windows. See the @@ -103,6 +110,35 @@ run `git diff --check` on your changes before you commit. [[describe-changes]] === Describe your changes well. +The log message that explains your changes is just as important as the +changes themselves. Your code may be clearly written with in-code +comment to sufficiently explain how it works with the surrounding +code, but those who need to fix or enhance your code in the future +will need to know _why_ your code does what it does, for a few +reasons: + +. Your code may be doing something differently from what you wanted it + to do. Writing down what you actually wanted to achieve will help + them fix your code and make it do what it should have been doing + (also, you often discover your own bugs yourself, while writing the + log message to summarize the thought behind it). + +. Your code may be doing things that were only necessary for your + immediate needs (e.g. "do X to directories" without implementing or + even designing what is to be done on files). Writing down why you + excluded what the code does not do will help guide future developers. + Writing down "we do X to directories, because directories have + characteristic Y" would help them infer "oh, files also have the same + characteristic Y, so perhaps doing X to them would also make sense?". + Saying "we don't do the same X to files, because ..." will help them + decide if the reasoning is sound (in which case they do not waste + time extending your code to cover files), or reason differently (in + 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 first line of the commit message should be a short description (50 characters is the soft limit, see DISCUSSION in linkgit:git-commit[1]), and should skip the full stop. It is also conventional in most cases to @@ -135,6 +171,13 @@ The body should provide a meaningful commit message, which: . alternate solutions considered but discarded, if any. +[[present-tense]] +The problem statement that describes the status quo is written in the +present tense. Write "The code does X when it is given input Y", +instead of "The code used to do Y when given input X". You do not +have to say "Currently"---the status quo in the problem statement is +about the code _without_ your change, by project convention. + [[imperative-mood]] Describe your changes in imperative mood, e.g. "make xyzzy do frotz" instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy @@ -144,8 +187,21 @@ without external resources. Instead of giving a URL to a mailing list archive, summarize the relevant points of the discussion. [[commit-reference]] -If you want to reference a previous commit in the history of a stable -branch, use the format "abbreviated hash (subject, date)", like this: + +There are a few reasons why you may want to refer to another commit in +the "more stable" part of the history (i.e. on branches like `maint`, +`master`, and `next`): + +. A commit that introduced the root cause of a bug you are fixing. + +. A commit that introduced a feature that you are enhancing. + +. A commit that conflicts with your work when you made a trial merge + of your work into `next` and `seen` for testing. + +When you reference a commit on a more stable branch (like `master`, +`maint` and `next`), use the format "abbreviated hash (subject, +date)", like this: .... Commit f86a374 (pack-bitmap.c: fix a memleak, 2015-03-30) @@ -259,9 +315,11 @@ 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. +sending out, please make sure it cleanly applies to the base you +have chosen in the "Decide what to base your work on" section, +and unless it targets the `master` branch (which is the default), +mark your patches as such. + [[send-patches]] === Sending your patches. @@ -365,7 +423,10 @@ Security mailing list{security-ml-ref}. 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. +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. :current-maintainer: footnote:[The current maintainer: gitster@pobox.com] :git-ml: footnote:[The mailing list: git@vger.kernel.org] |