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