diff options
author | Theodore Ts'o <tytso@mit.edu> | 2009-04-16 07:44:45 -0400 |
---|---|---|
committer | Jonathan Corbet <corbet@lwn.net> | 2009-06-04 12:34:33 -0400 |
commit | 2ae19acaa50a09c1099956efb895c0aca74ab050 (patch) | |
tree | 400180708dac8e250d9d2ad9dd2080d40b115d18 | |
parent | 5801da1b2f1207da21271ffd6768cd40a6c7f1c4 (diff) |
Documentation: Add "how to write a good patch summary" to SubmittingPatches
Unfortunately many patch submissions are arriving with painfully poor
patch descriptions. As a result of the discussion on LKML:
http://lkml.org/lkml/2009/4/15/296
explain how to submit a better patch description, in the (perhaps
vain) hope that maintainers won't end up having to rewrite the git
commit logs as often as they do today.
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Cc: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Jonathan Corbet <corbet@lwn.net>
-rw-r--r-- | Documentation/SubmittingPatches | 65 |
1 files changed, 51 insertions, 14 deletions
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 71b6da2b7175..6c456835c1fd 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches | |||
@@ -91,6 +91,10 @@ Be as specific as possible. The WORST descriptions possible include | |||
91 | things like "update driver X", "bug fix for driver X", or "this patch | 91 | things like "update driver X", "bug fix for driver X", or "this patch |
92 | includes updates for subsystem X. Please apply." | 92 | includes updates for subsystem X. Please apply." |
93 | 93 | ||
94 | The maintainer will thank you if you write your patch description in a | ||
95 | form which can be easily pulled into Linux's source code management | ||
96 | system, git, as a "commit log". See #15, below. | ||
97 | |||
94 | If your description starts to get long, that's a sign that you probably | 98 | If your description starts to get long, that's a sign that you probably |
95 | need to split up your patch. See #3, next. | 99 | need to split up your patch. See #3, next. |
96 | 100 | ||
@@ -492,12 +496,33 @@ phrase" should not be a filename. Do not use the same "summary | |||
492 | phrase" for every patch in a whole patch series (where a "patch | 496 | phrase" for every patch in a whole patch series (where a "patch |
493 | series" is an ordered sequence of multiple, related patches). | 497 | series" is an ordered sequence of multiple, related patches). |
494 | 498 | ||
495 | Bear in mind that the "summary phrase" of your email becomes | 499 | Bear in mind that the "summary phrase" of your email becomes a |
496 | a globally-unique identifier for that patch. It propagates | 500 | globally-unique identifier for that patch. It propagates all the way |
497 | all the way into the git changelog. The "summary phrase" may | 501 | into the git changelog. The "summary phrase" may later be used in |
498 | later be used in developer discussions which refer to the patch. | 502 | developer discussions which refer to the patch. People will want to |
499 | People will want to google for the "summary phrase" to read | 503 | google for the "summary phrase" to read discussion regarding that |
500 | discussion regarding that patch. | 504 | patch. It will also be the only thing that people may quickly see |
505 | when, two or three months later, they are going through perhaps | ||
506 | thousands of patches using tools such as "gitk" or "git log | ||
507 | --oneline". | ||
508 | |||
509 | For these reasons, the "summary" must be no more than 70-75 | ||
510 | characters, and it must describe both what the patch changes, as well | ||
511 | as why the patch might be necessary. It is challenging to be both | ||
512 | succinct and descriptive, but that is what a well-written summary | ||
513 | should do. | ||
514 | |||
515 | The "summary phrase" may be prefixed by tags enclosed in square | ||
516 | brackets: "Subject: [PATCH tag] <summary phrase>". The tags are not | ||
517 | considered part of the summary phrase, but describe how the patch | ||
518 | should be treated. Common tags might include a version descriptor if | ||
519 | the multiple versions of the patch have been sent out in response to | ||
520 | comments (i.e., "v1, v2, v3"), or "RFC" to indicate a request for | ||
521 | comments. If there are four patches in a patch series the individual | ||
522 | patches may be numbered like this: 1/4, 2/4, 3/4, 4/4. This assures | ||
523 | that developers understand the order in which the patches should be | ||
524 | applied and that they have reviewed or applied all of the patches in | ||
525 | the patch series. | ||
501 | 526 | ||
502 | A couple of example Subjects: | 527 | A couple of example Subjects: |
503 | 528 | ||
@@ -517,19 +542,31 @@ the patch author in the changelog. | |||
517 | The explanation body will be committed to the permanent source | 542 | The explanation body will be committed to the permanent source |
518 | changelog, so should make sense to a competent reader who has long | 543 | changelog, so should make sense to a competent reader who has long |
519 | since forgotten the immediate details of the discussion that might | 544 | since forgotten the immediate details of the discussion that might |
520 | have led to this patch. | 545 | have led to this patch. Including symptoms of the failure which the |
546 | patch addresses (kernel log messages, oops messages, etc.) is | ||
547 | especially useful for people who might be searching the commit logs | ||
548 | looking for the applicable patch. If a patch fixes a compile failure, | ||
549 | it may not be necessary to include _all_ of the compile failures; just | ||
550 | enough that it is likely that someone searching for the patch can find | ||
551 | it. As in the "summary phrase", it is important to be both succinct as | ||
552 | well as descriptive. | ||
521 | 553 | ||
522 | The "---" marker line serves the essential purpose of marking for patch | 554 | The "---" marker line serves the essential purpose of marking for patch |
523 | handling tools where the changelog message ends. | 555 | handling tools where the changelog message ends. |
524 | 556 | ||
525 | One good use for the additional comments after the "---" marker is for | 557 | One good use for the additional comments after the "---" marker is for |
526 | a diffstat, to show what files have changed, and the number of inserted | 558 | a diffstat, to show what files have changed, and the number of |
527 | and deleted lines per file. A diffstat is especially useful on bigger | 559 | inserted and deleted lines per file. A diffstat is especially useful |
528 | patches. Other comments relevant only to the moment or the maintainer, | 560 | on bigger patches. Other comments relevant only to the moment or the |
529 | not suitable for the permanent changelog, should also go here. | 561 | maintainer, not suitable for the permanent changelog, should also go |
530 | Use diffstat options "-p 1 -w 70" so that filenames are listed from the | 562 | here. A good example of such comments might be "patch changelogs" |
531 | top of the kernel source tree and don't use too much horizontal space | 563 | which describe what has changed between the v1 and v2 version of the |
532 | (easily fit in 80 columns, maybe with some indentation). | 564 | patch. |
565 | |||
566 | If you are going to include a diffstat after the "---" marker, please | ||
567 | use diffstat options "-p 1 -w 70" so that filenames are listed from | ||
568 | the top of the kernel source tree and don't use too much horizontal | ||
569 | space (easily fit in 80 columns, maybe with some indentation). | ||
533 | 570 | ||
534 | See more details on the proper patch format in the following | 571 | See more details on the proper patch format in the following |
535 | references. | 572 | references. |