Before diving into the best code review practices, I always ask this question –
Why do we need a Code Review process?
Code review process exists to improve the software development lifecycle (SDL). To improve the software and code that we are writing. This is done by adding an extra layer to the development process where your peers will take a look at the code to identify if there are any bugs, missing tests, code style or structure of the code before merging them to the main branch.
The meaning seems to be simple and understandable and probably makes sense. But I’ve observed that over time this code review process could become a lagger for the team. Instead of making the code a delight to write becomes some kind of exam that developers want to pass through to get their code to production.
I think code review should be looked at as a way to learn something new about your code.
The moment one starts thinking of the code review as a hurdle or an exam that you have to pass every time to take your code to production then the Code Review process has lost its meaning. You should never take review comments personally. I know we developers love our code but at the same time, we have to stay unattached to it. The moment we start attaching our emotions to the code, it starts to feel like friction.
Code review should be made as frictionless as possible for it to be useful.
There are 3 tips that I think could help you make the code review process fluent.
Table of Contents
1. Write A Clear Changelist Description
I always appreciate it when someone explains the WHY behind their code.
Whenever you are going to raise a pull request always write clear context behind that change. While writing the changelist, you should have the reviewer in mind. Not the context that you have in your mind but the context that will be useful for the reviewer.
I always try to write for the future developers who might visit older PRs to reads the changes, they should get the context within 5 minutes (given they know something about the work). Your changelist should include any background knowledge the reviewer/reader needs.
2. Separate Functional And Non Functional Change
There is always an urge to fix that extra space in the code or a missing line break. Maybe there is a missing indentation that should be fixed. These are non-functional changes.
When these changes are mixed with the functional changes then it becomes hard for your reviewer to identify which piece of code is responsible for changing the behaviour of the code and which one is just cosmetic.
No matter how bad the code is. Always separate functional changes from non-functional ones.
If you think the code is not in a good shape and requires refactoring then try to break this task into multiple steps instead of mixing them all at once.
- Add tests to cover the existing behaviour (if there aren’t any)
- Refactor the code structure keeping the tests
- This will provide security to your reviewer that the functionality is not broken
- Change the code and its respective tests
By following a step-by-step approach you will move faster.
3. Narrow Your Scope
The best change lists are the ones that do Just One Thing. There is a common pattern that I have seen growing up. The developer fixes a logic bug in the code but then they find a small glitch in a completely separate section of the code and they fixes that. Then they find something is wrong with the UI, they fix that.
This is called Scope Creep. A code-review anti-pattern.
If you are working on a particular bug fix. Just fix that bug. Even if it is a single character fix. Don’t do anything extra. If you find some other bugs or stuff, create different PRs for that.
This enables different members of your team to take on the review tasks in parallel. That way you move faster and get good comments.
Conclusion
Raising a good PR is as important as writing good code.
If you have written an excellent code but it is all over the place then it would take much more time to go through. Instead, write good and **precise** code, raise a PR. Write good code again, raise a PR. That’s the way to move forward faster and securely.
I hope you enjoyed reading.
Let me know in the comments below.