First, the main goal of a commit is to introduce an atomic change.
Atomic changes contain only one feature, refactor, fix, etc.
A good commit allows to evaluate one change at a time and it contains only/all the changes necessary.
Bad commits happen because of a bad workflow.
Your workflow is rarely a reflection of what should be contained in a good commit, i.e.
9 times out of 10, if you commit using git commit -a you are making it a 💩 commit.
The workflow should be:
- finish developing the feature and test it
- think back through all the changes you had to make and how you would want to present them
- stage selected lines/rebase to create commits that introduce them in logical chunks/order
If you have not already, please learn how to perform interactive staging.
http://git-scm.com/book/en/v2/Git-Tools-Interactive-Staging
Besides git, there are a ton of tools that provide GUI and make it is super simple. My favorite is Tower.
If you do the above, then your PR commits should read something along the lines of:
* feat: add claimUsername test helper
* test: ensure that username is at least 5 characters long
* test: ensure that in-use username cannot be claimed
* refactor: have registerNewTestUser use claimUsername
* refactor: have completeUserOnboarding use claimUsername
These commits are easy to review because each one of them introduces a change that can be reviewed and discussed in isolation.
Also, just by reading the commits, you gain a good mental map of all the changes.
It also helps to spot potentially breaking changes quicker.
- could adding a function break the code? no
- could adding a test break the code? no
- could refactoring existing code to use new feature break the code? maybe
It is also super important that your code works after every commit.
If it does not, then you cannot safely revert your codebase to an earlier point in time.
If you do not enforce the above, then you should enforce squashing commits when merging PRs.
Pro-tip: If you are interviewing for an engineering job, ask interviewer to share the recent commit log of the codebase that you will be contributing to.
git log --pretty=oneline --abbrev-commit
It will tell you a lot about the engineering culture they have.
In contrast, a bad commit log is a result of "commiting as you go" and looks more like:
* test: add helper
* refactor: use helper
* fix: add missing configuration
* fix: inline editing
* fix: failing tests
* fix: revert earlier fix
* refactor: use helper
These commits are bad because the changes they contain are non-atomic and non-linear. The only way for the reviewer to evaluate these changes is by looking at all files modified at once. That's a big mental burden to put on someone. ☹️
Note that I didn't talk about contents/formatting of the commit messages themselves. What convention you adopt is a lot less important than just having atomic commits. In fact, great commit messages are a by-product/indication of atomic commits and vice-versa.
And in case you are thinking "this is a lot of work", "no one is reading these" or "it slows me down", then you should re-evaluate the sports that you are playing. Engineering is a team sport. Great engineering input compounds.
You don't need every commit/PR to be perfect, but it shows who is putting effort, and if others see you put in effort, they will do to. 🕊
For more great content of mine, follow me on Twitter https://twitter.com/kuizinas/status/1541496585275727875
Top comments (0)