Daily Group Code Reviews - Increasing Efficiency

While many junior Devs struggle with code reviews, both when performing and receiving them, I've found that even the more senior team members sometimes struggle with comments left on their code. The reasons for this are various, with junior devs not having the experience to know what they're looking for and comments being left often reading as criticism rather than the constructive feedback as which it was meant.

Before I go any further, I want to say that this blog post is inspired by a post I made on LinkedIn about the same topic. My LinkedIn post got quite a response so I figured I'd go into a bit more detail here on my blog.

Looking back on my own time as a junior/mid-level developer, I don't recall anyone ever teaching me how to perform a code review. Having spoken to a few different senior developers who came from other backgrounds than my own, I found this reflection echoed. None of us could even pinpoint at what moment in our career we actually started to be expected to do them, which was certainly odd. A former coworker and I even recalled one prior boss who wouldn't recognise any PR unless he was the one to personally review it (talk about a bottleneck).

As a Principal Lead Developer, these days I have my own team and I have the freedom to try things out that I think will help them. I expect the whole team to perform reviews, to avoid bottlenecks like the one above and to help the more junior members learn as performing code reviews is one of the best ways to get confident with a code base. Unfortunately, during times of high workload, I have still occasionally found PRs building up unreviewed.

All of this is to explain why I recently started running a daily meeting with all of the devs on my team, directly after our daily standup. In this meeting, dubbed the "PR Cycle", we run through a couple of PRs as a group. Each day, we ask a different developer to run the meeting (selected at random by spinning a wheel) and they share their screen as they review a PR. As part of reviewing, they explain to the team which key issues they are looking for and provide further detail conversationally whenever they add a requested change. We still add the key notes in a comment on Github to avoid anything being forgotten.

I've known devs to write incredibly long messages, sometimes even including apologies, on change requests in an attempt to avoid offence - which both buries the requested change and also takes a lot longer to write. We've found that when we request changes in a meeting instead, it's very difficult to take it personally. It also provides the person who raised the PR an opportunity to ask questions or even discuss the change with the wider team. The person doing the reviews shouldn't be the only one speaking after all.

While the number of comments on PRs has increased overall, the amount of needed back and forth on a ticket has steeply declined and as such PRs spend less time waiting to be reviewed overall. In fact, this meeting has been so successful that our most active repo has dropped from an average of 8 open PRs at any one time to an average of just 1.4 without any drop in the number of tickets being completed. Some days we may get through 2 or 3 reviews whereas on others we may only get through one due to it being a large change but because we're making the time for it as a group, the number of PRs awaiting review still drops.

I've also had feedback from the senior developers on my team who have indicated that something that one of the other devs has mentioned looking out for has changed their own PR reviewing behaviour. So when it comes time to get the two approvals we require to merge something, we're catching more that could otherwise have come back to bite us.

The PR Cycle Meeting has been remarkably successful but it's not the only thing we do to ensure high standards on our tribe. We maintain a good CI pipeline including various Unit, Integration and Security tests. We use PR templates which ensures easy access to information for anyone reviewing the code (which will also block merging if the included checklist is not complete). We also make sure that no code is written without first being documented to a level that allows the team to understand the intended change.

This is to say, that the PR Cycle meeting may not work for everyone but, if you have a few juniors on your team or find yourself having a large PR backlog, it may be worth giving a go.