9 Tips on The Bottleneck of Code Reviews

When fellow CTO Academy member André asked the community for assistance with the bottleneck of code reviews, it sent the cogs whirring.

It’s a problem I’m facing with my own teams and have been for some months. So it was almost comforting to hear another tech leader facing the same issues.

I had experienced efficient code review practices before, so the question led me to articulate what had worked in the past.

Here are some thoughts … (thanks also to Nathan Danielsen and Ahmet Kara for their input to this article)

Well Specified Tickets

It was a large cooperation, so the team had a project manager and business analysts involved that would make sure each ticket was well refined with a clear scope.

This had the benefit of the developer assigned to the ticket knowing exactly what to develop and that anyone collecting the ticket for code review knew the scope of what they needed to look at.

Not all teams have the luxury of a business analyst and project manager but where possible, try and bring more than just technical input into ticket specification where it might help.

Keep Work Atomic

Keep the scope of tickets and pull requests in check.

This kept pull requests small. It reduced side effects of the work being done and made reviewing the code itself much faster.

Pull requests with >1000 lines of changes are a red flag and no one is going to read through that many lines of code.

Aim For Complete Code Coverage With Tests

Include both unit tests and integration/end to end testing.

Every new piece of work should have associated unit and integration tests as part of the review. This enables a high amount of confidence in the work being reviewed and allows for easy automation.

Ensure time for this testing has been included in your estimates and sprint planning.

“Right to Left” Working

We had a sprint board with the columns “ready for dev”, “in progress”, “peer review”, and “done”.

We adopted a process of “Right to Left” working.

It’s tempting as a developer to create a PR, drag your ticket to the peer review column on the sprint board, and go immediately left to see what’s in ready for dev. Instead, we’d go right.

When you submitted a piece of work, you first checked the peer review column.

If there was work there that needed reviewing, you did it BEFORE looking in ready for dev.

Automated Linting (ND) and Static Analysers

Make heavy use of linting / style checking tools, especially in a pre-commit hook.

Styling feedback on CRs isn’t a good use of time and static analysis tools can provide even deeper insights in to code quality such as amount of duplication, code smells, and the aforementioned code coverage.

Code Review Guidelines (ND)

Set up some code review guidelines on what a good code review is and what isn’t.

Sometimes a checklist has been helpful in normalising what requires a look and what doesn’t.

Using markdown in the pull request, and Github’s (or equivalent) PR templates automate the writing process. Include a “how to test” section, including new dependencies, database migrations, or reproduction steps.

This stops the reviewer fumbling around in the dark to even get the branch up and running.

Not All Reviews Are Created Equal (ND)

Does that one line change really need two developers for approval?

Security and pricing changes will need more/more senior review over small CSS changes.

Pairing Up (ND)

Instead of having async code reviews, try having engineers pair up to look at CRs together, or alternatively pair up on writing the code in the first place.

While this on the face of it doubles the man hours required on a piece of work, the level of knowledge sharing, and increase in quality of code makes it a worthwhile investment.

Time

Ensure time is given for code review in your estimates.

In many teams it’s all too common for every possible man hour of the sprint was filled with tickets.

If you have 5 people on a team, and each day is two points, it’s tempting to fill the sprint with 100 points of tickets. No time for meetings, discussion, or code reviews. In the team where the review process worked well, we calculated the number of points the team could handle in a sprint and immediately cut it in half.

This allowed team members to feel like they had TIME to code review, and correctly set expectations for management.

Treat code reviews like an investment. Investing an hour of time now to avoid 3 hours of rework later. Use metrics to track the number of story points tackled in a sprint.

If its high, but code reviews are not being done, dial it back. Give 20% time to code reviews and revaluate your metrics after a trial period.

Where its worked, it was also a team that was very process orientated as a whole. So, no one thing acted as a silver bullet for solving the problem. I certainly don’t profess to be an expert, but that team was the best experience I’ve had as a developer and I can count on one hand the number of times I undertook rework.

The struggle I’m finding now, is building habit.

We’re a young team, both in our time together and levels of experience.

Getting the team to build the habit of, and confidence to code review is proving to be an ever-present challenge.

We’ll be implementing many of these solutions as we move forward, ensuring buy-in from team each step of the way.

Written by Andrew Ryan – Engineering Manager @ Intuety.io