Development
Top 10 do's and don'ts for effective code review
Context
I recently had the opportunity to work on a highly technical and complex project with a team consisting of 5 full-time developers and a tech lead. We had various degrees of familiarity with the project, some of us having worked on the project since its inception for years while the rest had anywhere between 1 to 12 months on it. The project was challenging due to its highly technical domain and codebase complexity which had grown over the years from increase of the project’s scope and multiple necessary architectural changes and rewrites. At the same time, we had a successful software product that exceeded stakeholder expectations. Our roadmap continued to be full of brand-spanking new features to develop.
Challenge
As is the case with Osedea development teams, we had a fairly high implementation velocity. It was not uncommon for 10-12 core pull requests to be published daily between the 6 of us. Where the challenge came was in getting those PRs reviewed and merged. While ad-hoc PR reviews worked well with a 2 developer team, my start on the project coincided with the team tripling in size in the span of one month. This meant the check-as-you-go code review bottleneck took full effect - increased Pull Request longevity and inconsistent reviews. In this article, we’ll review the steps we took to turn code review from a hurdle back to the powerful tool it’s meant to be.
Don’ts
- Don’t skip refinement sessions as a team (hah this being our first point and not directly related to code review…). Refinement sessions are pivotal in making sure that devs have at the very least a high-level understanding of all the stories in a sprint and that knowledge silos are kept to a minimum. This is especially important in a highly technical and complex project with developers of various degrees of knowledge and experience in it. Code review will feel hard when you have no idea what’s going on.
- Don’t nitpick. If you do, prepend with “Nit:” In code review, you want to keep the focus on the important feedback and requests for change. Nits bring noise and also dishearten. As well, a lot of nits tend to relate to formatting - use automated linting tools and checks in your CI pipeline if you have one.
- Don’t sleep on live code review sessions especially for more involved PRs. You want to avoid inefficient ping-ponging in PR comments. If reading a PR is bringing up a lot of questions and feedback, go ahead and ask for a quick sync with the author. Live discussions are just more productive and oftentimes more friendly… which brings us to the next point.
- Don’t let ego come into your code review. We are professionals, craftspeople, and most importantly teammates. Enough said?
- Don’t skip PR descriptions, tags, etc. Decide as a team on a PR template and standards for opening PRs. These don’t have to be lengthy or complicated (in fact the simpler the better), but these little features are there for you to boost code review productivity.
Do’s
- Do keep PRs small and contained. Again, refinement sessions are important here since well-defined stories and subtasks make this a lot easier to do. Brief PRs are easier to read and review well. On the author’s side, they guard against scope creep and allow for less bugs to be introduced as the PR evolves with any feedback given.
- Do block time devoted to code review daily in your schedule. Because of our large team, we found that it was most efficient to have two code review time blocks in our days. For most of us, we’d look at PRs once in the morning and once in the afternoon. Find the right amount for your context and stay consistent.
- Do review your own PRs before asking others to review them. Make sure you’ve written a decent PR description and have used any relevant tags. If possible, take a short break after coding and then review the diff yourself with your critical cap on. You’ll be surprised how many things you can catch yourself without others needing to point them out to you.
- Do ask questions when you don’t understand something in a PR. Clean code should be readable and so if you’re confused, the author should know. As well, PRs and code review are an excellent form of knowledge sharing.
- Do give both constructive and positive feedback. We all know that constructive feedback is pivotal to code quality and the individual growth of developers. Positive feedback, however, is also essential for these things. When you spot good code quality and good work done by a teammate, point it out and celebrate it! Doing so encourages not only the PR author to continue to strive for that quality and work, but also teaches and reminds other reviewers to do the same.
Conclusion
We hope our 10 Do’s and Don’ts are helpful to you and your projects. We close with this final reflection: good code review comes hand in hand with good team culture. It's important to grow trust and respect in and outside of the code as well as cultivate strong care for both code quality and team(mate) growth in order to build great working software. If you're looking to build innovative software with a team that values quality and collaboration, get in touch with us today.
Did this article start to give you some ideas? We’d love to work with you! Get in touch and let’s discover what we can do together.