Commit Organization Best Practices#

Caution

This guide has been based on LSST’s Git guide. Please respect the creators and contributor of the original guide as they have done an external work.

Commits should represent discrete logical changes to the code#

OpenStack has an excellent discussion of commit best practices; this is recommended reading for all developers. This section summarizes those recommendations.

Commits on a ticket branch should be organized into discrete, self-contained units of change. In general, we encourage you to err on the side of more granular commits; squashing a pull request into a single commit is an anti-pattern. A good rule-of-thumb is that if your commit summary message needs to contain the word and, there are too many things happening in that commit.

Associating commits to a single logical change makes debugging and code audits easier:

  • git bisect is more effective for zeroing in on the change that introduced a regression.

  • git blame is more helpful for explaining why a change was made.

  • Better commit organization guides reviewers through your pull request, making for more effective code reviews.

  • A bad commit can more easily be reverted later with fewer side-effects.

Some edits serve only to fix white space or code style issues in existing code. Those whitespace and style fixes should be made in separate commits from new development. Usually it makes sense to fix whitespace and style issues in code before embarking on new development (or rebase those fixes to the beginning of your ticket branch).

Rebase commits from code reviews rather than having ‘review feedback’ commits#

Code review will result in additional commits that address code style, documentation and implementation issues. Where possible, authors should rebase (i.e., git rebase -i main) their ticket branch to squash the post-review fixes to the pre-review commits. The preference is that a pull request, when merged, should have a coherent development story and look as if the code was written correctly the first time.

If such a rebase is too difficult (e.g., because it would result in excessive merge conflicts), then post-review commits may be left appended to the pre-review commits. Any commits not squashed in this way should represent discrete logical changes and have informative commit messages, as if the changes had been made before review.