Code review is the process of checking over code that has been submitted by another developer to spot any mistakes and to ensure that the code does what it is supposed to do. It’s like quality assurance for code.
They are a great way to improve the productivity of your team. Code reviews dramatically reduce the amount of time spent fixing simple mistakes. The sooner a bug is fixed, the less it costs in real money to fix. Code review has business value.
Something that I’ve noticed in all of the teams that I’ve worked on is that most developers just read through the pull request in the browser. However, I perform code reviews locally because I want to notice more mistakes in less time. In addition, I want to be a productive reviewer, oh, and to make my life easy!
The Code Review process
How to submit a code review
Assuming that you are using git:
- Take a new branch for each piece of work that you do.
- Commit to that branch then push the changes up.
- If you are using Bitbucket, when you push from the command line you are given a link to create a pull request right from the terminal!
- Click the “Create Pull Request” (PR) button and then send the link of the pull request to your team.
- To speed up reviews, leave a comment with:
- A link to the story or task;
- The title of the task/story or short description;
- A list of instructions of how to manually test this.
How to approach a code review
How would you do it?
First, read the story or task. Think about how you might approach the problem.
Do a quick functional test
Check out the branch locally, and open the code in your Integrated Development Environment (IDE) or code editor.
Run the code to see if, at a high level, the application with the new code does what it is supposed to.
Try switching back to master (or the branch that this will be merged into) to confirm how the application worked before and after.
Read the code
Next, look at each of the changed files. I like to use the list of files on the PR webpage to do this. Another way to do this using your IDE is to use the compare with branch feature if your IDE supports this.
Take the first pass of each file to check if there are any warnings or errors. Then read and understand the code. I like to click on and into variables and methods to see how they have been used. I like to play around with the code as if it was my own.
Make helpful comments and suggestions on the PR webpage on anything that you notice that is out of place.
If you are unsure about whether something needs changed, write the comment as a question. For example, “did you leave this logging statement for testing purposes?” instead of “DELETE” or “NOT USED ANYWHERE”.
Don’t be too harsh. Your goal isn’t to raise the code from a grade C to grade A* but to go from say C to a B. Maybe after the first round of comments raise by another grade. Remember, the code does what it says that it does!
Before pressing comment, consider if your comment is an opinion or a useful suggestion. I’ve fought many unnecessary battles over whether loads of variables really make code more readable.
If you notice that the same “mistake” is occurring over and over, don’t comment it over and over again. Go and ask the person, or make a comment on the pull request itself. This way the developer doesn’t have to defend their work multiple times. Even if it is not your intention, each comment is a hole poked in their masterpiece.
Why should I perform code reviews locally?
I don’t know about you, but I spend a lot more time looking at code in the bright colours in my IDE than I do at black text on a web page.
I’ve my machine set up the way I like it. I can spot code mistakes without even having to read it just by the colours of underline or the shape of the code.
Your IDE can spot things to do with the code change in relation to the codebase as a whole. You can’t this in a browser-based code review.
Common mistakes that I spot in my IDE that I don’t notice in the browser as easily are:
- Variable created but not used
- Variable created and immediately overwritten
- Unused method
- Spelling mistakes
- Missing null check
- Copy and pasted code
- Debugger statements (JS)
Some other advice on code reviews
Reading code is tiring, even if it has been well written. You are reading code that someone else wrote and trying to understand what they were thinking.
Take breaks. If you find you that you start skimming over the code, walk away. Fill up your water or make a coffee. There is no point looking at the code if you are not giving it your full attention.
Make use of the Pomodoro technique! See my post on the pomodoro technique here. This is a great way to make sure that you take breaks!
Also, don’t try and do too many code reviews at once. As mentioned above, you will get tired or frustrated. The longer you are looking at the code, the fewer mistakes you will spot.
In conclusion, code reviews are a great way to improve the quality of your code. However, if you want to increase the number of mistakes you spot in a smaller amount of time, you should look at the code in your IDE locally. Be a productive reviewer!
What are some common mistakes you find in code reviews? Comment below!