Ultra Efficient Code Reviews Through a Simple Three-Pass Method
Code reviews are integral part of SDLC. If you are a professional software engineer, there is a high chance that you have reviewed someone's code and someone has reviewed your code.
Why do we need a code review ? Put simply , it allows teams to maintain high code quality and in fact improve it as time goes by. But the benefits of code reviews don't end there. Code reviews allow teams to keep code consistency in terms of standardized programming patterns, improve code readability and maintainability, reduce buggy / half-baked code, reduce tech debt, improve code health etc. Here's an funny illustration on code quality.
Now that you have some context ( not that you didn't , but just in case ) , I would like to deep dive into today's topic - "Code Review Speed" !
The speed of code review is a key metric that every agile team should track. Its the average time taken for reviewing a PR in your team. Ideally the average code review time should be less than a day, but more often than not you will see code reviews taking two,three and sometimes even more days. Slower code reviews generally translate into reduced team velocity, low quality code reviews, sprint spillovers and wait-agony for engineers.
In our organization, I personally saw and experienced the adverse effects of slower code reviews. And thats where I started following a three-pass method to really speed up my code reviews. Let's go through these 3 passes one by one.
1. Language constructs and coding style
In my first pass of code review, I start with assessing the changes based on basic language / programming standards and the coding style which is being followed by the team.
Generally the kind of loopholes I am looking for are - any hardcoded strings which could be moved to constants, opportunities where we can use latest/better programming constructs to simplify the code ( eg. in JS code using ?. instead of null checks, using || to replace a if statement, using reduce over map/filter ), reusable code candidates, missing validation checks for variables, missing documentation, missing exception handling, explicit type checks, typos and spelling mistakes, incorrect naming for function and variables, missing RTL support etc.
This pass helps me to identify any quick fixes in the change that needs to be done. This pass basically identifies code cleanup opportunities.
2. Solution design and implementation approach
In the second pass, I would look at the solution or change as whole. I start looking into thought process put-in behind the change as whole.
Generally the kind of loopholes I am looking for in this phase are - programming anti patterns (e.g. putting some business logic in client vs in backend service), programming overkills (e.g oversimplified business logic), design gaps ( eg. logic built to work only in specific context & could fail in different context), backward compatibility issues, logging & analytics gaps, proper error boundary handling.
This pass helps me to identify potential show stoppers. These are major gaps which can potentially affect codebase health leading to bug spread in future.
3. Test coverage, device compatibility
In the last pass, I focus on testing and compatibility aspects. Programmers are inherently biased towards their own code. Most of us are focused on getting things working and forget to test the robustness and error handling capability for our code. That's where this pass helps.
Generally the kind of loopholes I am looking for in this phase are - missing test cases, incomplete or badly designed test cases, lack of testing across multiple device sizes and platforms, incomplete localization support etc.
This final pass helps me to call out any last minute important misses. More often than not I always find few things in this pass of review.
Well that's all I had for this article. If this approach makes sense to you, I would definitely recommend to give it a try.