Scope
The best practices below refer to commits that are ready to merge to master, although it may be useful to apply them to "development" commits on branches as well.
Organizing changes into commits
(See
Jira | ||||||||
---|---|---|---|---|---|---|---|---|
|
Follow the practice for structural organization of commits described in the OpenStack Best Practices document. From the linked document:
The cardinal rule for creating good commits is to ensure there is only one "logical change" per commit. There are many reasons why this is an important rule:
- The smaller the amount of code being changed, the quicker & easier it is to review & identify potential flaws.
- If a change is found to be flawed later, it may be necessary to revert the broken commit. This is much easier to do if there are not other unrelated code changes entangled with the original commit.
- When troubleshooting problems using GIT's bisect capability, small well defined changes will aid in isolating exactly where the code problem was introduced.
- When browsing history using GIT annotate/blame, small well defined changes also aid in isolating exactly where & why a piece of code came from.
Frequently, code review will result in additional commits. Authors should feel free to rebase the commits related to the review comments to squash them with the relevant pre-review commit. Said another way, authors should make the commit history look like the code was written correctly the first time. Review comments will be stored in the pull request with an out-of-date commit.
Things to avoid when creating commits:
- Mixing whitespace changes with functional code changes
- Mixing two unrelated functional changes
- Sending large new features in a single giant commit
- Having a single "addressing review comments" commit
...
title | Rule of Thumb |
---|
...
...
Warning | ||
---|---|---|
| ||
Authors: rebase as necessary to satisfy the above. |
Contents and format of commit messages
(See
Jira | ||||||||
---|---|---|---|---|---|---|---|---|
|
Follow the practice described in a blog post by Tim Pope (or the very similar Perl commit guidelines, excerpted in the first comment to DM-1182).
Example from the blog post:
Capitalized, short (50 chars or less) summary More detailed explanatory text, if necessary. Wrap it to about 72 characters or so. In some contexts, the first line is treated as the subject of an email and the rest of the text as the body. The blank line separating the summary from the body is critical (unless you omit the body entirely); tools like rebase can get confused if you run the two together. Write your commit message in the imperative: "Fix bug" and not "Fixed bug" or "Fixes bug." This convention matches up with commit messages generated by commands like git merge and git revert. Further paragraphs come after blank lines. - Bullet points are okay, too - Typically a hyphen or asterisk is used for the bullet, followed by a single space, with blank lines in between, but conventions vary here - Use a hanging indent
...
title | Authors and Reviewers |
---|
...