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

Implement a business logic
Update tests
Fix code review issues
Doc cleanup
            

Implement subtraction in a calculating service
Add missing tests of a subtraction for the calculating service
Refactor the subtraction service with a command handler
Remove obsolete endpoints from the documentation 
            

Squash all unnecessary commits

Do not clutter the history with unnecessary commits that could be squashed/fixed-up.

Bad Really bad Better also OK

Implement a business logic
Implement a business logic
Implement a business logic
Implement a business logic
Update tests
Update tests
Update documentation
            

Implement a business logic
Implement a business logic
Implement a business logic
Implement a business logic
Implement a business logic
Implement a business logic
Implement a business logic
Implement a business logic
            

Implement a business logic
Update tests
Update documentation
            

Implement a business logic - update calculation service
Implement a business logic - UI layout
Update tests - add missing tests for the calculation service
Update tests - calculator UI
Update documentation
            

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

Implement subtraction in a calculating service
Add missing tests of a subtraction for the calculating service
Implement subtraction in a calculating service
Add missing tests of a subtraction for the calculating service
Add missing tests of a subtraction for the calculating service
Update documentation
Add missing tests of a subtraction for the calculating service
            

Implement subtraction in a calculating service
Add missing tests of a subtraction for the calculating service
Update documentation
            

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

Implement subtraction in a calculating service
            

[CALC-123] Implement subtraction in a calculating service
            

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

Fix the annoying UI bug that made people crazy
            

Fix the unclickable button bug reported in [UI-1234]
            

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.

  1. Define what the proper commit message is, like: [LABEL] Commit message with a label and first upper latter.
  2. Define a regexp for that structure
  3. 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

Leave a Reply

Avatar placeholder

Your email address will not be published. Required fields are marked *