There are lots of good practices often repeated regarding the code review process, however I very rarely see anything regarding reviewing the structure and the content of commit messages.
The proper history of the change is as important as the code itself and can really help to understand the purpose of the implementation, especially after some time. A good commit description and a logical order can be a blessing, especially when finding a bug in a legacy project in the middle of a night.
Below there is my checklist what I try to do with my commits before I submit the pull request.
For the sake of readability, I will order commits from top to bottom, like in a Git rebase editor.
Describe your commits appropriately
Simple as that. The more descriptive is the commit, the better. Especially after some time when we need to travel back in time.
Bad | Better |
---|---|
|
|
Squash all unnecessary commits
Do not clutter the history with unnecessary commits that could be squashed/fixed-up.
Bad | Really bad | Better | also OK |
---|---|---|---|
|
|
|
|
Keep your commits in order if possible
Before you create the pull request, it is beneficial to see whether the commit structure tells a story. If not, amend the commit history and put commit messages in a logical order.
Of course, as with all changes to the commit structure, it is wise to do it carefully to avoid unnecessary reaching the git reflog
for help.
Bad | Better |
---|---|
|
|
To modify the commit history, use git rebase
functionality (either directly from a command line or i.e. from IntelliJ IDEA).
Below there is a history of last 6 commits in my blog and the history could look better. To amend the history, just change define what to do with each commit (pick, reword, edit, squash, etc.) and reorder them when needed:
$ git rebase -i HEAD~6
1 pick 6bebd7f Update post about code quality automation
2 pick 8c4b51f Add video to automate code quality post
3 pick 148905b Add posts about reviewing commits
4 pick 10d2408 Update posts about reviewing commits
5 pick 863a9a3 Code review your commits - update post
6 pick 7055040 Code review your commits - update post
7
8 # Przestawianie 8d4b113..7055040 na 10d2408 (6 poleceń)
9 #
10 # Polecenia:
11 # p, pick <zapis> = dobierz zapis
12 # r, reword <zapis> = użyj zapisu, ale przeredaguj jego komunikat
13 # e, edit <zapis> = użyj zapisu, ale zatrzymaj się, żeby go poprawić
14 # s, squash <zapis> = użyj zapisu, ale połącz go z poprzednim (spłaszcz)
15 # f, fixup <zapis> = jak „squash”, ale odrzuć komunikat tego zapisu
16 # x, exec <polecenie> = wykonaj polecenie (resztę wiersza) w powłoce
17 # b, break = zatrzymaj się tu (kontynuuj przestawianie przez „git rebase --continue”)
18 # d, drop <zapis> = usuń zapis
19 # l, label <etykietka> = nazwij bieżące HEAD
20 # t, reset <etykietka> = zresetuj HEAD do etykietki
21 # m, merge [-C <zapis> | -c <zapis>] <etykietka> [# <wiersz>]
22 # . utwórz zapis scalenia używając pierwotnego komunikatu scalenia
23 # . (albo <wiersza>, jeśli nie podano pierwotnego zapisu scalenia.
24 # . Użyj -c <zapis>, aby przeredagować komunikat zapisu
If possible, link the commit to the dedicated tracking ticket
Having a more descriptive Jira (or any other tool) ticket is great when we need to travel back in time. Of course, providing that the Jira ticket is more descriptive that a commit itself and does not end up with a title and empty description 😉
Bad | Better |
---|---|
|
|
Establish a labeling system for changes without any external reference
Sometimes we just want to do a simple and quick change without any bureaucratic overhead (like creating pointless tracking tickets). Any labeling system is better than nothing. We can label such commits like [TECH], [FIX], [DOC], etc. – they give the overall understanding of the commit purpose after a quick glimpse.
[CALC-123] Implement subtraction in a calculating service
[CALC-123] Add missing tests of a subtraction for the calculating service
[CALC-123] Remove obsolete endpoints from the documentation
[TECH] Update the version of the Spring boot
[DOC] Update showing CI badges
Avoid colloquialisms
This is so obvious that one could argue whether I should talk about that at all. Surprisingly, I have seen few such cases in my life. And because devs focus more on the code than commit structure/wording, I have seen such commits landing in the Client repos. This was pretty embarrassing. Even if you never dare to use colloquialisms in your code/commits, do not be so sure that others think the same way 🙂
Bad | Better |
---|---|
|
|
Use git hooks to validate the commit message structure
There’s nothing better than automating the boring stuff and checking a proper Git message structure is a great candidate.
- Define what the proper commit message is, like:
[LABEL] Commit message with a label and first upper latter
. - Define a regexp for that structure
- Use the regexp in the git-hooks locally and apply same constraint in the remote Git repository to validate all incoming commits.
Below there is a script I often use to validate the commit structure. It will require a proper label in front of the message and will skip checks of merge and revert commits.
#!/usr/bin/env sh
check=$(head -1 $1 | egrep '(^\[(\w*?-[0-9]+|TECH|FIX|DOC)\]\s.+$) | (^Merge.+$) | (^Revert.+$)')
if [ "" = "$check"]; then
echo "Invaid commit message." 1>&2
echo "The proper commit structure examples:" 1>&2
echo "[JIRA-000] Commit message" 1>&2
echo "[TECH] Commit message" 1>&2
echo "[FIX] Commit message" 1>&2
echo "[DOC] Commit message" 1>&2
echo "... other messages if needed" 1>&2
exit 1
fi
Rebase your local branch instead of constantly merging the master/develop
Last but not least, I think it is much cleaner to rebase your current branch on top of the master/develop than constantly merging that. Merge commits can be filtered out while browsing the history but if we can get rid the vast majority of them, it is even better.
Summary
There are far more good practices used to keep the commit history beautiful. However, at least in my case, above simple steps are usually enough to maintain things in order. Above all, I try to treat commit history like a regular code and it always pays off after some time.
0 Comments