Tips for Working with Topic Branches
-
Divide the changes introduced by a topic branch into separate commits, so that every commit contains a single logical change. There are no hard rules for what comprises a "single logical change", so you will need to apply common sense. Generally speaking, try hard to spare the reviewer the need to "switch context" while looking at various parts of a single commit - e.g. if you mix variable renaming with other code cleanups, the reviewer needs to make an educated guess about your intentions for every single line of the diff; meanwhile, reviewing a commit renaming some variable even in thousands of lines is simple and can often even be automated.
-
In order to make it easy for the reviewer to track changes introduced since the last review round, do not rebase the topic branch unless any of the following applies:
- the topic branch is signed off on and is about to get merged,
- the topic branch depends on a change which was merged into the base branch in the meantime,
- the reviewer asked you to.
Tips for Writing Commit Messages
Subject Lines
-
Make sure each commit has a subject line that is separated from the rest of the commit log message with an empty line. Not adhering to this rule can cause
git commit --fixupto create commits with horribly long subject lines. -
Try to fit the subject line of every commit in 50 characters. This is encouraged by
man git-commit. -
Do not use trailing punctuation in commit subject lines.
-
Do not put issue numbers in commit subject lines - use a proper branch name prefix instead.
Log Message Body
-
Explain what problem a given commit solves or what new feature it adds.
-
Try hard to avoid being overly terse and/or vague - the reader is very unlikely to be acquainted with the context and/or motivation behind your work. As the author of a change, it is your duty to explain why it is needed - the reviewer is not supposed to be making guesses about what you are trying to achieve.
-
If applicable, briefly discuss alternative solutions which were also considered, but ultimately rejected.
-
If a given commit addresses warnings generated by compilers, static analyzers, etc., include the warnings being addressed in the commit message body. See commit
abfde3d543for an example. -
Read https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst#n102 for further inspiration.