When you're ready to commit, be sure to write a Good Commit Message™. What's in a Good Commit Message™?
We follow most general engineering best practices from other projects using Git. Some additional guidelines specific to CockroachDB apply.
General best practices:
(must) Keep in mind who reads: think of the reviewers & future maintainers, think of the release notes
(recommended) Keep the commit title concise but information-rich
(recommended) Use the imperative mood in the subject line, but not in the body
(recommended) Wrap the body at a consistent width under 100 characters
(suggested) Do not end the subject line with a period
Specific to CockroachDB:
(must) Include a release note annotation, in the right position
(recommended) Prefix the subject line with the affected package/area
Table of contents
General structure of a commit message
The first line of the commit message is the commit title.
Tooling around git give this line a special role. The title can be at most one line. Do not break it across multiple lines. Separate it from the next paragraph with an empty line, otherwise other tools will become confused.
Prefix your commit subject line with the affected package, if one can easily be chosen. For example, the subject line of a commit mostly affecting the
serverpackage might read: "server: use net.Pipe instead of TCP HTTP/gRPC connections". Commits which affect many packages as a result of a shared dependency change should probably begin their subjects with the name of the shared dependency. Finally, some commits may need to affect many packages in a way which does not point to a specific package; those commits may begin with "
*:" or "
all:" to indicate their reach.
Since the title is prefixed with a package name, don't capitalize the title. In English, the text that follows a colon (
:) is not capitalized.
Do not end the title with a period. It's a title, not a sentence.
Not every commit requires both a title and a body. Sometimes a single line is fine, especially when the change is so simple that no further context is necessary. For example:
Nothing more need be said; if the reader wonders what the typo was, she can simply take a look at the change itself.
Note however that at least one of the commits in a PR should include a release note annotation in a commit body, if only to state "Release note: none" to declare there are no user-facing changes. See
Make the commit title short. Other projects have a guideline of trying to keep it around 50 characters. Keeping subject lines short ensures that they are readable in
git log --pretty=oneline, and forces the author to think for a moment about the most concise way to explain what's going on.
Tip: If you’re having a hard time summarizing, you might be committing too many changes at once. Strive for atomic commits.
Chris Beams: A well-crafted Git commit message is the best way to communicate context about a change to fellow developers (and indeed to their future selves). A diff will tell you what changed, but only the commit message can properly tell you why. Peter Hutterer makes this point well:
Re-establishing the context of a piece of code is wasteful. We can’t avoid it completely, so our efforts should go to reducing it [as much] as possible. Commit messages can do exactly that and as a result, a commit message shows whether a developer is a good collaborator.
How to write a good commit description
Learn about how commit messages help the reviewers and future maintainers in our PR organization philosophy.
Think about the following questions:
Why should this change should be made? What is wrong with the current code? Explain how things were prior to the patch.
Why should it be done in this particular way?
Did you try a different way before? Describe alternate approaches you considered.
Can a reviewer confirm that it works as intended?
What are the consequences for further users of this code? Do they need to understand the code in a new way?
Was this discussed before? Add a link to an existing public issue if it exists, to provide context. (Note that the commit should still be self-explanatory even if there is a linked issue.)
Avoid URLs in commit messages.
Use git hashes instead of URLs to refer to other changes.
If there was external documentation or discussions, also summarize the relevant points.
Wrap the message body so that lines are less than 100 characters long. There are too many tools that cannot re-flow paragraphs, or do a miserable job at it, to afford using unwrapped paragraphs in commit messages.
Suggestion: use your editor's standard wrapping width of 72 characters so as to make commit messages uniform across the entire project. Many of your fellow developers also use the default configuration of their editor that will wrap at 72 columns.
If a URL is too long, give it its own line, but don't break or wrap it.
Pay attention to
OpenStack's chapter Information in commit messages:
Do not assume the reviewer understands what the original problem was.
When reading bug reports, after a number of back & forth comments, it is often as clear as mud, what the root cause problem is. The commit message should have a clear statement as to what the original problem is. The bug is merely interesting historical background on /how/ the problem was identified. It should be possible to review a proposed patch for correctness without needing to read the bug ticket.
Do not assume the reviewer has access to external web services/site.
In 6 months time when someone is on a train/plane/coach/beach/pub troubleshooting a problem & browsing Git history, there is no guarantee they will have access to the online bug tracker, or online blueprint documents. The great step forward with distributed SCM is that you no longer need to be "online" to have access to all information about the code repository. The commit message should be totally self-contained, to maintain that benefit.
Do not assume the code is self-evident/self-documenting.
What is self-evident to one person, might be clear as mud to another person. Always document what the original problem was and how it is being fixed, for any change except the most obvious typos, or whitespace only commits.
Describe why a change is being made.
A common mistake is to just document how the code has been written, without describing /why/ the developer chose to do it that way. By all means describe the overall code structure, particularly for large changes, but more importantly describe the intent/motivation behind the changes.
Read the commit message to see if it hints at improved code structure.
Often when describing a large commit message, it becomes obvious that a commit should have in fact been split into 2 or more parts. Don't be afraid to go back and rebase the change to split it up into separate commits.
Ensure sufficient information to decide whether to review.
When [...] email alerts [are sent] for new patch submissions there is minimal information included, principally the commit message and the list of files changes. Given the high volume of patches, it is not reasonable to expect all reviewers to examine the patches in detail. The commit message must thus contain sufficient information to alert the potential reviewers to the fact that this is a patch they need to look at.
The first commit line is the most important.
In Git commits the first line of the commit message has special significance. It is used as email subject line, git annotate messages, gitk viewer annotations, merge commit messages and many more places where space is at a premium. As well as summarizing the change itself, it should take care to detail what part of the code is affected. eg if it affects the libvirt driver, mention 'libvirt' somewhere in the first line.
Describe any limitations of the current code.
If the code being changed still has future scope for improvements, or any known limitations then mention these in the commit message. This demonstrates to the reviewer that the broader picture has been considered and what tradeoffs have been done in terms of short term goals vs. long term wishes.
Do not include patch set-specific comments.
In other words, if you rebase your change please don't add "Patch set 2: rebased" to your commit message. That isn't going to be relevant once your change has merged. Please do make a note of that in [the PR comments] as a comment on your change, however. It helps reviewers know what changed between patch sets. This also applies to comments such as "Added unit tests", "Fixed localization problems", or any other such patch set to patch set changes that don't affect the overall intent of your commit.
Place the references to bugs or github issues after the explanation, but before the release note. For example: