1.. _docs-pw-style-commit-message: 2 3==================== 4Commit message style 5==================== 6Pigweed commit message bodies and summaries are limited to 72 characters wide to 7improve readability, and should be prefixed with the name of the module that the 8commit is affecting. The commits should describe what is changed, and why. When 9writing long commit messages, consider whether the content should go in the 10documentation or code comments instead. For example: 11 12.. code-block:: none 13 14 pw_some_module: Short capitalized description 15 16 Details about the change here. Include a summary of what changed, and a clear 17 description of why the change is needed. Consider what parts of the commit 18 message are better suited for documentation or code. 19 20 - Added foo, to fix issue bar 21 - Improved speed of qux 22 - Refactored and extended qux's test suite 23 24----------------------------- 25Include both "what" and "why" 26----------------------------- 27It is important to include a "why" component in most commits. Sometimes, why is 28evident - for example, reducing memory usage, optimizing, or fixing a bug. 29Otherwise, err on the side of over-explaining why, not under-explaining why. 30 31When adding the "why" to a commit, also consider if that "why" content should go 32into the documentation or code comments. 33 34.. admonition:: **Yes**: Reasoning in commit message 35 :class: checkmark 36 37 .. code-block:: none 38 39 pw_sync_xrtos: Invoke deadlock detector 40 41 During locking, run the deadlock detector if there are enough cycles. 42 Though this costs performance, several bugs that went unnoticed would have 43 been caught by turning this on earlier. Take the small hit by default to 44 better catch issues going forward; see extended docs for details. 45 46.. admonition:: **No**: Reasoning omitted 47 :class: error 48 49 .. code-block:: none 50 51 pw_sync_xrtos: Invoke deadlock detector 52 53 During locking, run the deadlock detector if there are enough cycles. 54 55.. _docs-pw-style-commit-message-imperative: 56 57--------------- 58Imperative mood 59--------------- 60Use `imperative mood <https://en.wikipedia.org/wiki/Imperative_mood>`_ 61in the commit message title. Do not use third-person singular present tense. 62In general, the commit message title should start with a verb, and that verb 63should not end in ``s``. 64 65.. tab-set:: 66 67 .. tab-item:: Yes 68 69 pw_string: Update docs 70 71 .. tab-item:: No 72 73 pw_string: Updates docs 74 75--------------------------------------- 76Documentation instead of commit content 77--------------------------------------- 78Consider whether any of the commit message content should go in the 79documentation or code comments and have the commit message reference it. 80Documentation and code comments are durable and discoverable; commit messages 81are rarely read after the change lands. 82 83.. admonition:: **Yes**: Created docs and comments 84 :class: checkmark 85 86 .. code-block:: none 87 88 pw_i2c: Add and enforce invariants 89 90 Precisely define the invariants around certain transaction states, and 91 extend the code to enforce them. See the newly extended documentation in 92 this change for details. 93 94.. admonition:: **No**: Important content only in commit message 95 :class: error 96 97 .. code-block:: none 98 99 pw_i2c: Add and enforce invariants 100 101 Add a new invariant such that before a transaction, the line must be high; 102 and after, the line must be low, due to XXX and YYY. Furthermore, users 103 should consider whether they could ever encounter XXX, and in that case 104 should ZZZ instead. 105 106--------------------------- 107Lists instead of paragraphs 108--------------------------- 109Use bulleted lists when multiple changes are in a single change. Ideally, try to 110create smaller changes so this isn't needed, but larger changes are a practical 111reality. 112 113.. admonition:: **Yes**: Uses bulleted lists 114 :class: checkmark 115 116 .. code-block:: none 117 118 pw_complicated_module: Pre-work for refactor 119 120 Prepare for a bigger refactor by reworking some arguments before the larger 121 change. This change must land in downstream projects before the refactor to 122 enable a smooth transition to the new API. 123 124 - Add arguments to MyImportantClass::MyFunction 125 - Update MyImportantClass to handle precondition Y 126 - Add stub functions to be used during the transition 127 128.. admonition:: **No**: Long paragraph is hard to scan 129 :class: error 130 131 .. code-block:: none 132 133 pw_foo: Many things in a giant BWOT 134 135 This change does A, B, and C. The commit message is a Big Wall Of Text 136 (BWOT), which we try to discourage in Pigweed. Also changes X and Y, 137 because Z and Q. Furthermore, in some cases, adds a new Foo (with Bar, 138 because we want to). Also refactors qux and quz. 139 140------------ 141Subject line 142------------ 143The subject line (first line of the commit) should take the form ``pw_<module>: 144Short description``. The module should not be capitalized, but the description 145should (unless the first word is an identifier). See below for the details. 146 147.. admonition:: **No**: Uses a non-standard ``[]`` to indicate module: 148 :class: error 149 150 .. code-block:: none 151 152 [pw_foo]: Do a thing 153 154.. admonition:: **No**: Has a period at the end of the subject 155 :class: error 156 157 .. code-block:: none 158 159 pw_bar: Do something great. 160 161.. admonition:: **No**: Puts extra stuff after the module which isn't a module. 162 :class: error 163 164 .. code-block:: none 165 166 pw_bar/byte_builder: Add more stuff to builder 167 168Multiple modules 169================ 170Sometimes it is necessary to change code across multiple modules. 171 172#. **2-5 modules**: Use ``{}`` syntax shown below 173#. **>5 modules changed** - Omit the module names entirely 174#. **Changes mostly in one module** - If the commit mostly changes the 175 code in a single module with some small changes elsewhere, only list the 176 module receiving most of the content 177 178.. admonition:: **Yes**: Small number of modules affected; use {} syntax. 179 :class: checkmark 180 181 .. code-block:: none 182 183 pw_{foo, bar, baz}: Change something in a few places 184 185 When changes cross a few modules, include them with the syntax shown 186 above. 187 188.. admonition:: **Yes**: Many modules changed 189 :class: checkmark 190 191 .. code-block:: none 192 193 Change convention for how errors are handled 194 195 When changes cross many modules, skip the module name entirely. 196 197.. admonition:: **No**: Too many modules changed for subject 198 :class: error 199 200 .. code-block:: none 201 202 pw_{a, b, c, d, e, f, g, h, i, j}: Change convention for how errors are handled 203 204 When changes cross many modules, skip the module name entirely. 205 206Non-standard modules 207==================== 208Most Pigweed modules follow the format of ``pw_<foo>``; however, some do not, 209such as targets. Targets are effectively modules, even though they're nested, so 210they get a ``/`` character. 211 212.. admonition:: **Yes**: 213 :class: checkmark 214 215 .. code-block:: none 216 217 targets/xyz123: Tweak support for XYZ's PQR 218 219 PQR is needed for reason ZXW; this adds a performant implementation. 220 221Capitalization 222============== 223The text after the ``:`` should be capitalized, provided the first word is not a 224case-sensitive symbol. 225 226.. admonition:: **No**: Doesn't capitalize the subject 227 :class: error 228 229 .. code-block:: none 230 231 pw_foo: do a thing 232 233 Above subject is incorrect, since it is a sentence style subject. 234 235.. admonition:: **Yes**: Doesn't capitalize the subject when subject's first 236 word is a lowercase identifier. 237 :class: checkmark 238 239 .. code-block:: none 240 241 pw_foo: std::unique_lock cleanup 242 243 This commit message demonstrates the subject when the subject has an 244 identifier for the first word. In that case, follow the identifier casing 245 instead of capitalizing. 246 247 However, imperative style subjects often have the identifier elsewhere in 248 the subject; for example: 249 250 .. code-block:: none 251 252 pw_foo: Improve use of std::unique_lock 253 254------ 255Footer 256------ 257We support a number of `git footers`_ in the commit message, such as ``Bug: 258123`` in the message below: 259 260.. code-block:: none 261 262 pw_something: Add foo and bar functions 263 264 Bug: 123 265 266The footer syntax is described in the `git documentation 267<https://git-scm.com/docs/git-interpret-trailers>`_. Note in particular that 268multi-line footers are supported: 269 270.. code-block::none 271 272 pw_something: Add foo and bar functions 273 274 Test: Carried out manual tests of pw_console 275 as described in the documentation. 276 277You are encouraged to use the following footers when appropriate: 278 279* ``Bug``: Associates this commit with a bug (issue in our `bug tracker`_). The 280 bug will be automatically updated when the change is submitted. When a change 281 is relevant to more than one bug, include multiple ``Bug`` lines, like so: 282 283 .. code-block:: none 284 285 pw_something: Add foo and bar functions 286 287 Bug: 123 288 Bug: 456 289 290* ``Fixed`` or ``Fixes``: Like ``Bug``, but automatically closes the bug when 291 submitted. 292 293 .. code-block:: none 294 295 pw_something: Fix incorrect use of foo 296 297 Fixes: 123 298 299* ``Test``: The author can use this field to tell the reviewer how the change 300 was tested. Typically, this will be some combination of writing new automated 301 tests, running automated tests, and manual testing. 302 303 Note: descriptions of manual testing procedures belong in module 304 documentation, not in the commit message. Use the ``Test`` field to attest 305 that tests were carried out, not to describe the procedures in detail. 306 307 .. code-block:: none 308 309 pw_something: Fix incorrect use of foo 310 311 Test: Added a regression unit test. 312 313In addition, we support all of the `Chromium CQ footers`_, but those are 314relatively rarely useful. 315 316.. _bug tracker: https://bugs.chromium.org/p/pigweed/issues/list 317.. _Chromium CQ footers: https://chromium.googlesource.com/chromium/src/+/refs/heads/main/docs/infra/cq.md#options 318.. _git footers: https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/git-footers.html 319