Skip to content

Commit fb9e91b

Browse files
authored
REVIEW: Add detailed commit message guidelines (#171)
The commit section had only a brief mention of imperative mood and why-not-what. Expand it with concrete guidance on subject format, body content, and anti-patterns to avoid. Also add the top-level REVIEW.md symlink so the relative link from AGENTS.md resolves correctly (matching how AGENTS.md is set up). Assisted-by: OpenCode (Claude Opus 4) Signed-off-by: Colin Walters <walters@verbum.org>
1 parent 5b85b92 commit fb9e91b

3 files changed

Lines changed: 46 additions & 16 deletions

File tree

REVIEW.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
common/REVIEW.md

common/AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ include an `Assisted-by: TOOLNAME (MODELNAME)`. For example,
2525
## Code guidelines
2626

2727
The [REVIEW.md](REVIEW.md) file describes expectations around
28-
testing, code quality, commit organization, etc. If you're
28+
testing, code quality, commit messages, commit organization, etc. If you're
2929
creating a change, it is strongly encouraged after each
3030
commit and especially when you think a task is complete
3131
to spawn a subagent to perform a review using guidelines (alongside

common/REVIEW.md

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -90,24 +90,54 @@ follow your reasoning: "Especially grateful for breaking it up into individual
9090
commits so I can more easily follow your train of thought."
9191

9292
Preparatory refactoring should be separate from behavioral changes. Each commit
93-
should tell a clear story and be reviewable independently. Commit messages should
94-
explain the "why" not just the "what," and use imperative mood ("Add feature"
95-
not "Added feature").
93+
should tell a clear story and be reviewable independently. Where applicable,
94+
create "prep" commits that could be merged separately from the behavioral change.
9695

97-
### PR Descriptions
96+
### Commit Messages
97+
98+
Write clear and descriptive commit messages using a `component: Summary`
99+
subject, such as `kernel: Add find API w/correct hyphen-dash equality, add docs`.
100+
Use imperative mood: "Add integration with..." not "Adds integration with...".
101+
102+
The body of the commit should start with at least one sentence (or paragraph)
103+
describing **why** the change is being made, even for something apparently
104+
trivial. For example a "refactor" commit might have a "why" rationale of just
105+
"Prep for handling X later." A big commit introducing a feature may seem
106+
self-explanatory, but there is often ambient context like "A large-scale Debian
107+
user wanted this" that provides helpful grounding in the motivation.
108+
109+
If there's a linked tracking issue, often that will contain a more extensive
110+
rationale that doesn't need to be duplicated entirely in the commit message,
111+
but do ensure the commit message has something useful on its own for a rationale.
112+
113+
Keep it natural and concise. A few sentences of prose explaining the design
114+
intent or the high-level data flow is often good enough. If there's a
115+
non-obvious consequence of the change, call it out briefly (e.g. "Note the
116+
manifest becomes part of the GC root") rather than explaining the full
117+
mechanism. Think about what a reviewer needs to know that may not be obvious
118+
from a skim of the code.
119+
120+
Do not restate obvious parts of what is already visible in the commit diff:
98121

99-
PRs should link to the issues they address using `Closes:` or `Fixes:` with
100-
full URLs. One reviewer noted: "I edited this issue just now to have
101-
`Closes: <URL>` but let's try to be sure we're doing that kind of thing in
102-
general in the future."
122+
- "Changed function X to call Y"
123+
- Generic `Changes:` sections with bulleted lists of implementation details
124+
- "Files changed" sections — completely redundant with git
103125

104-
Document known limitations and caveats explicitly. When approaches have tradeoffs
105-
or don't fully solve a problem, say so. For complex investigations, use collapsible
106-
`<details>` sections to include debugging notes without cluttering the main
107-
description.
126+
Implementation details belong in the code documentation. The goal of the
127+
commit message is like a "cover letter" for the change, with a primary
128+
rationale of why the change is being made, alongside a concise summary of
129+
its implementation.
108130

109-
Think about broader implications: "But we'll have this problem across all repos
110-
right?" Consider how your change affects the wider ecosystem.
131+
Another thing that can go in the commit message is brief descriptions
132+
of alternative approaches that were considered and discarded.
133+
134+
Closes: tags should generally come at the end of the commit message.
135+
136+
### PR Descriptions
137+
138+
Generally, just restate the commit message.
139+
140+
Where it makes sense, it is OK to include additional details though.
111141

112142
### Keeping PRs Current
113143

@@ -123,7 +153,6 @@ Do not add `Signed-off-by` lines automatically—these require explicit human
123153
action after review. If code was AI-assisted, include an `Assisted-by:` trailer
124154
indicating the tool and model used.
125155

126-
127156
## Architecture and Design
128157

129158
### Workarounds vs Proper Fixes

0 commit comments

Comments
 (0)