Commit message style#
Pigweed commit message bodies and summaries are limited to 72 characters wide to improve readability, and should be prefixed with the name of the module that the commit is affecting. The commits should describe what is changed, and why. When writing long commit messages, consider whether the content should go in the documentation or code comments instead. For example:
pw_some_module: Short capitalized description
Details about the change here. Include a summary of what changed, and a clear
description of why the change is needed. Consider what parts of the commit
message are better suited for documentation or code.
- Added foo, to fix issue bar
- Improved speed of qux
- Refactored and extended qux's test suite
Include both “what” and “why”#
It is important to include a “why” component in most commits. Sometimes, why is evident - for example, reducing memory usage, optimizing, or fixing a bug. Otherwise, err on the side of over-explaining why, not under-explaining why.
When adding the “why” to a commit, also consider if that “why” content should go into the documentation or code comments.
Yes: Reasoning in commit message
pw_sync_xrtos: Invoke deadlock detector
During locking, run the deadlock detector if there are enough cycles.
Though this costs performance, several bugs that went unnoticed would have
been caught by turning this on earlier. Take the small hit by default to
better catch issues going forward; see extended docs for details.
No: Reasoning omitted
pw_sync_xrtos: Invoke deadlock detector
During locking, run the deadlock detector if there are enough cycles.
Present imperative#
Use present imperative style instead of passive descriptive.
Yes: Uses imperative style for subject and text.
pw_something: Add foo and bar functions
This commit correctly uses imperative present-tense style.
No: Uses non-imperative style for subject and text.
pw_something: Adds more things
Use present tense imperative style for subjects and commit. The above
subject has a plural "Adds" which is incorrect; should be "Add".
Documentation instead of commit content#
Consider whether any of the commit message content should go in the documentation or code comments and have the commit message reference it. Documentation and code comments are durable and discoverable; commit messages are rarely read after the change lands.
Yes: Created docs and comments
pw_i2c: Add and enforce invariants
Precisely define the invariants around certain transaction states, and
extend the code to enforce them. See the newly extended documentation in
this change for details.
No: Important content only in commit message
pw_i2c: Add and enforce invariants
Add a new invariant such that before a transaction, the line must be high;
and after, the line must be low, due to XXX and YYY. Furthermore, users
should consider whether they could ever encounter XXX, and in that case
should ZZZ instead.
Lists instead of paragraphs#
Use bulleted lists when multiple changes are in a single CL. Ideally, try to create smaller CLs so this isn’t needed, but larger CLs are a practical reality.
Yes: Uses bulleted lists
pw_complicated_module: Pre-work for refactor
Prepare for a bigger refactor by reworking some arguments before the larger
change. This change must land in downstream projects before the refactor to
enable a smooth transition to the new API.
- Add arguments to MyImportantClass::MyFunction
- Update MyImportantClass to handle precondition Y
- Add stub functions to be used during the transition
No: Long paragraph is hard to scan
pw_foo: Many things in a giant BWOT
This CL does A, B, and C. The commit message is a Big Wall Of Text
(BWOT), which we try to discourage in Pigweed. Also changes X and Y,
because Z and Q. Furthermore, in some cases, adds a new Foo (with Bar,
because we want to). Also refactors qux and quz.
Subject line#
The subject line (first line of the commit) should take the form pw_<module>:
Short description. The module should not be capitalized, but the description
should (unless the first word is an identifier). See below for the details.
No: Uses a non-standard [] to indicate module:
[pw_foo]: Do a thing
No: Has a period at the end of the subject
pw_bar: Do something great.
No: Puts extra stuff after the module which isn’t a module.
pw_bar/byte_builder: Add more stuff to builder
Multiple modules#
Sometimes it is necessary to change code across multiple modules.
2-5 modules: Use
{}syntax shown below>5 modules changed - Omit the module names entirely
Changes mostly in one module - If the commit mostly changes the code in a single module with some small changes elsewhere, only list the module receiving most of the content
Yes: Small number of modules affected; use {} syntax.
pw_{foo, bar, baz}: Change something in a few places
When changes cross a few modules, include them with the syntax shown
above.
Yes: Many modules changed
Change convention for how errors are handled
When changes cross many modules, skip the module name entirely.
No: Too many modules changed for subject
pw_{a, b, c, d, e, f, g, h, i, j}: Change convention for how errors are handled
When changes cross many modules, skip the module name entirely.
Non-standard modules#
Most Pigweed modules follow the format of pw_<foo>; however, some do not,
such as targets. Targets are effectively modules, even though they’re nested, so
they get a / character.
Yes:
targets/xyz123: Tweak support for XYZ's PQR
PQR is needed for reason ZXW; this adds a performant implementation.
Capitalization#
The text after the : should be capitalized, provided the first word is not a
case-sensitive symbol.
No: Doesn’t capitalize the subject
pw_foo: do a thing
Above subject is incorrect, since it is a sentence style subject.
Yes: Doesn’t capitalize the subject when subject’s first word is a lowercase identifier.
pw_foo: std::unique_lock cleanup
This commit message demonstrates the subject when the subject has an
identifier for the first word. In that case, follow the identifier casing
instead of capitalizing.
However, imperative style subjects often have the identifier elsewhere in the subject; for example:
pw_foo: Improve use of std::unique_lock