Code reviews are the glue of any effective software engineering team. A code review is the stage at which an engineer requests their changes to be merged into the main development branch. During a code review, other teammates and senior leadership can comment on and suggest changes to your code through version control systems such as Git and GitLab.
Code reviews are the inflection point from an individual engineer’s changes into a team-wide contribution towards a central code base.
The most important piece of code reviews lies within this transition. Code reviews start with individual ownership from one or two developers and end with team ownership. Once that code has been merged, any future bugs or problems are not an individual’s problem — they are a team problem.
In the age of fully remote technology teams, code reviews matter now more than ever. It’s extremely important to take the proper amount of time (bonus tip) to review a teammate’s code. A team that has effective code reviews gains the advantages of adhering to industry standards, onboarding new developers faster, and creating lasting software, just to name a few.
With all this being said, being competent in your code reviews is critical to becoming a proficient software engineer. If you’re like me and too often just hit “Approve,” then it’s time for us to get our act together and learn five rules for every code review!
1. Always Share Your Thoughts
As obvious as it sounds, you have to be critically aware of your thoughts when you’re participating in a code review. If you’re a newbie and don’t understand what the code is doing, you need to voice your confusion. In this situation, you may want to contact your teammates over your messaging system before commenting on the merge request.
However, I’ve also seen countless senior developers repeatedly ask for clarification. The solution could be as simple as adding a comment for some byte manipulation, or it could involve an entire refactoring of an algorithm. It needs to make sense to the team.
Imagine if all of the senior devs at your company decided that they wouldn’t say anything if they didn’t understand the code in a merge request. That would be a complete nightmare! This is why it’s important — especially for junior devs — to voice your thoughts. If you never ask, you’ll never learn!
Regardless of the result, it’s not a bad thing to show your confusion. It is a bad thing to leave your confusion unanswered.
2. Understand the Acceptance Criteria (AC)
This rule goes two ways. First, you obviously need to know the purpose and goals of the merge request. Second, you need to understand how those goals are satisfied by the changes made. When it comes to understanding the AC, it’s all about drilling down each layer of abstraction.
A first step you could take to understand the AC would be to review the ticket associated with the merge request. From there, the ticket should be able to describe the high-level goals and implementation details.
If you’re the author of the MR, you could leave further details along with any gotchas in the description of your merge request. Maybe you even go as far as to leave your own comments on your MR for any confusing pieces of code, making the lives of your teammates easier!
Conversely, if you’re reviewing the MR, it’s just as important that you understand the AC. Do you want to know what doesn’t help you understand the code? Sifting through the change list on GitLab. Instead, pull down the branch itself and tinker with the new changes. Experiment, run test cases, or check and see if the code base takes unusually long to build or execute. If you find anything wrong or are confused, make sure you leave a comment in the code review and follow rule #1.
3. Keep Your Changes As Small as Possible
Personally, I find it brutally depressing to open a merge request with 1,000+ lines of code in it. No one on your team is going to look through that much code — period. Ideally, your MR should be between ten and 100 lines of code.
While that may seem daunting at first, there are practical steps to encourage a concise code review. The first step is to make sure your .gitignore file is in order. This file alerts Git’s version control system of any files that should be ignored. They could be relevant to your IDE or based on dependencies that resulted from a web framework such as Angular.
Another tip you can immediately use is to promote daily code reviews. If you’re using GitLab, you can change the title of your MRs to start with “WIP” (Work In Progress) or “Draft.” This allows your team to begin examining and commenting on your code changes, but with the restriction that you cannot merge into the desired branch.
Daily code reviews increase visibility for your team on the changes being made each day. This allows your team to avoid gigantic code reviews where no one really knows what’s happening and promotes more team-wide ownership and understanding of your code bases.