1 commit 8de6badd32fb584d60733a6236113edba00f8701
2 Author: Willy Tarreau <w@1wt.eu>
3 Date: Fri Jul 26 15:21:54 2019 +0200
5 DOC: improve the wording in CONTRIBUTING about how to document a bug fix
7 Insufficiently described bug fixes are still too frequent. It's a real
8 pain to create each new maintenance release, as 3/4 of the time is spent
9 trying to guess what problem a patch fixes, which is already important
10 in order to decide whether to pick the fix or not, but is even more
11 capital in order to write understandable release notes.
13 Christopher rightfully demands that a patch tagged "BUG" MUST ABSOLUTELY
14 describe the problem and why this problem is a bug. Describing the fix
15 is one thing but if the bug is unknown, why would there be a fix ? How
16 can a stable maintainer be convinced to take a fix if its author didn't
17 care about checking whether it was a real bug ? This patch tries to
18 explain a bit better what really needs to appear in the commit message
19 and how to describe a bug.
21 To be backported to all relevant stable versions.
23 (cherry picked from commit 41f638c1eb8167bb473a6c8811d7fd70d7c06e07)
24 Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
26 diff --git a/CONTRIBUTING b/CONTRIBUTING
27 index 0fcd921e..201e122d 100644
30 @@ -454,7 +454,18 @@ do not think about them anymore after a few patches.
32 11) Real commit messages please!
34 - Please properly format your commit messages. To get an idea, just run
35 + The commit message is how you're trying to convince a maintainer to adopt
36 + your work and maintain it as long as possible. A dirty commit message almost
37 + always comes with dirty code. Too short a commit message indicates that too
38 + short an analysis was done and that side effects are extremely likely to be
39 + encountered. It's the maintainer's job to decide to accept this work in its
40 + current form or not, with the known constraints. Some patches which rework
41 + architectural parts or fix sensitive bugs come with 20-30 lines of design
42 + explanations, limitations, hypothesis or even doubts, and despite this it
43 + happens when reading them 6 months later while trying to identify a bug that
44 + developers still miss some information about corner cases.
46 + So please properly format your commit messages. To get an idea, just run
47 "git log" on the file you've just modified. Patches always have the format
48 of an e-mail made of a subject, a description and the actual patch. If you
49 are sending a patch as an e-mail formatted this way, it can quickly be
50 @@ -506,9 +517,17 @@ do not think about them anymore after a few patches.
52 But in any case, it is important that there is a clean description of what
53 the patch does, the motivation for what it does, why it's the best way to do
54 - it, its impacts, and what it does not yet cover. Also, in HAProxy, like many
55 - projects which take a great care of maintaining stable branches, patches are
56 - reviewed later so that some of them can be backported to stable releases.
57 + it, its impacts, and what it does not yet cover. And this is particularly
58 + important for bugs. A patch tagged "BUG" must absolutely explain what the
59 + problem is, why it is considered as a bug. Anybody, even non-developers,
60 + should be able to tell whether or not a patch is likely to address an issue
61 + they are facing. Indicating what the code will do after the fix doesn't help
62 + if it does not say what problem is encountered without the patch. Note that
63 + in some cases the bug is purely theorical and observed by reading the code.
64 + In this case it's perfectly fine to provide an estimate about possible
65 + effects. Also, in HAProxy, like many projects which take a great care of
66 + maintaining stable branches, patches are reviewed later so that some of them
67 + can be backported to stable releases.
69 While reviewing hundreds of patches can seem cumbersome, with a proper
70 formatting of the subject line it actually becomes very easy. For example,
71 @@ -630,13 +649,23 @@ patch types include :
73 - BUG fix for a bug. The severity of the bug should also be indicated
74 when known. Similarly, if a backport is needed to older versions,
75 - it should be indicated on the last line of the commit message. If
76 - the bug has been identified as a regression brought by a specific
77 - patch or version, this indication will be appreciated too. New
78 - maintenance releases are generally emitted when a few of these
79 - patches are merged. If the bug is a vulnerability for which a CVE
80 - identifier was assigned before you publish the fix, you can mention
81 - it in the commit message, it will help distro maintainers.
82 + it should be indicated on the last line of the commit message. The
83 + commit message MUST ABSOLUTELY describe the problem and its impact
84 + to non-developers. Any user must be able to guess if this patch is
85 + likely to fix a problem they are facing. Even if the bug was
86 + discovered by accident while reading the code or running an
87 + automated tool, it is mandatory to try to estimate what potential
88 + issue it might cause and under what circumstances. There may even
89 + be security implications sometimes so a minimum analysis is really
90 + required. Also please think about stable maintainers who have to
91 + build the release notes, they need to have enough input about the
92 + bug's impact to explain it. If the bug has been identified as a
93 + regression brought by a specific patch or version, this indication
94 + will be appreciated too. New maintenance releases are generally
95 + emitted when a few of these patches are merged. If the bug is a
96 + vulnerability for which a CVE identifier was assigned before you
97 + publish the fix, you can mention it in the commit message, it will
98 + help distro maintainers.
100 - CLEANUP code cleanup, silence of warnings, etc... theoretically no impact.
101 These patches will rarely be seen in stable branches, though they
git.99rst.org / openwrt-packages.git / blob