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

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