path: root/docs/style/commit_message.rst

.. _docs-pw-style-commit-message:

====================
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:

.. code-block:: none

  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.

.. admonition:: **Yes**: Reasoning in commit message
   :class: checkmark

   .. code-block:: none

      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.

.. admonition:: **No**: Reasoning omitted
   :class: error

   .. code-block:: none

      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.

.. admonition:: **Yes**: Uses imperative style for subject and text.
   :class: checkmark

   .. code-block:: none

      pw_something: Add foo and bar functions

      This commit correctly uses imperative present-tense style.

.. admonition:: **No**: Uses non-imperative style for subject and text.
   :class: error

   .. code-block:: none

      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.

.. admonition:: **Yes**: Created docs and comments
   :class: checkmark

   .. code-block:: none

      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.

.. admonition:: **No**: Important content only in commit message
   :class: error

   .. code-block:: none

      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.

.. admonition:: **Yes**: Uses bulleted lists
   :class: checkmark

   .. code-block:: none

      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

.. admonition:: **No**: Long paragraph is hard to scan
   :class: error

   .. code-block:: none

      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.

.. admonition:: **No**: Uses a non-standard ``[]`` to indicate module:
   :class: error

   .. code-block:: none

      [pw_foo]: Do a thing

.. admonition:: **No**: Has a period at the end of the subject
   :class: error

   .. code-block:: none

      pw_bar: Do something great.

.. admonition:: **No**: Puts extra stuff after the module which isn't a module.
   :class: error

   .. code-block:: none

      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

.. admonition:: **Yes**: Small number of modules affected; use {} syntax.
   :class: checkmark

   .. code-block:: none

      pw_{foo, bar, baz}: Change something in a few places

      When changes cross a few modules, include them with the syntax shown
      above.

.. admonition:: **Yes**: Many modules changed
   :class: checkmark

   .. code-block:: none

      Change convention for how errors are handled

      When changes cross many modules, skip the module name entirely.

.. admonition:: **No**: Too many modules changed for subject
   :class: error

   .. code-block:: none

      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.

.. admonition:: **Yes**:
   :class: checkmark

   .. code-block:: none

      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.

.. admonition:: **No**: Doesn't capitalize the subject
   :class: error

   .. code-block:: none

      pw_foo: do a thing

      Above subject is incorrect, since it is a sentence style subject.

.. admonition:: **Yes**: Doesn't capitalize the subject when subject's first
   word is a lowercase identifier.
   :class: checkmark

   .. code-block:: none

      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:

   .. code-block:: none

     pw_foo: Improve use of std::unique_lock

------
Footer
------
We support a number of `git footers`_ in the commit message, such as ``Bug:
123`` in the message below:

.. code-block:: none

   pw_something: Add foo and bar functions

   Bug: 123

You are encouraged to use the following footers when appropriate:

* ``Bug``: Associates this commit with a bug (issue in our `bug tracker`_). The
  bug will be automatically updated when the change is submitted. When a change
  is relevant to more than one bug, include multiple ``Bug`` lines, like so:

  .. code-block:: none

      pw_something: Add foo and bar functions

      Bug: 123
      Bug: 456

* ``Fixed`` or ``Fixes``: Like ``Bug``, but automatically closes the bug when
  submitted.

  .. code-block:: none

      pw_something: Fix incorrect use of foo

      Fixes: 123

In addition, we support all of the `Chromium CQ footers`_, but those are
relatively rarely useful.

.. _bug tracker: https://bugs.chromium.org/p/pigweed/issues/list
.. _Chromium CQ footers: https://chromium.googlesource.com/chromium/src/+/refs/heads/main/docs/infra/cq.md#options
.. _git footers: https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/git-footers.html