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 change. Ideally, try to create smaller changes so this isn’t needed, but larger changes 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 change 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.

  1. 2-5 modules: Use {} syntax shown below

  2. >5 modules changed - Omit the module names entirely

  3. 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