/BLOG

So as we briefly recapped in the intro, pull request (PR) reviews offer a number of benefits. We won’t go into details here, there are a lot of blog posts and videos out there discussing why doing PR reviews is considered to be a best practice. It’s important to note that a PR review is not a technique, it’s a process (agreement) within a team to review each others code. Let’s see what we need to consider to make this process a success.

Find out why you review

Hopefully, your team agreed on this because they see potential benefits in spending time reviewing code. This might sound like a no-brainer, but we’ve found that numerous teams do reviews “because they need to”. It’s a mandatory requirement, made by an architecture board or maybe some manager or product owner who likes to be technically involved. It’s very likely that in these cases, the quality of a review might not be that great.

So to start off with, discuss within your team why you think PR reviews are important. List the benefits that apply to your particular situation, since context is important.

Make clear what reviews will cost

Also, discuss with the entire team what the cost of performing reviews will be. This is an important point since people like product owners, scrum masters and other stakeholders might need to made aware of the fact that serious code reviews take time. Time well spent, that is.

Discuss when you review your code

Being a process agreement, a PR review can take place at any given time within the process. You can opt to review each and every commit or you can review release branches only. Or maybe agree to review once a year. Although I have an opinion on all of these options (being not great), I don’t think there’s an agreement which fits each and every team. So each team should decide for themselves what works best.

In most teams I’ve worked with, some sort of feature branching is used. It usually makes sense to review the code as part of the feature branch pull request. But again: make this a team decision and have the agility to change the agreement when needed.

Take reviews seriously

Another smell of immature teams: a sizeable PR is reviewed within a couple of minutes after the author requested it. In other words: the review was not thorough and it’s likely that things which needed correcting (maybe bugs) went by unseen. If this is the case you can seriously ask yourself: is it worth the trouble and wait? When you don’t take reviews seriously, than why do them at all? Quality probably won’t improve.

Providing feedback

This might be one of the most challenging things doing code reviews within your team: giving and receiving feedback. On the giving side, as a reviewer you should try to give useful feedback to the author. Consider the following:

• Be nice. Another no-brainer probably, but we’ve seen this go wrong more than we’d like to admit. Even I sometimes catch myself writing comments that in retrospect were not that friendly.
• Provide alternatives / solutions. Don’t simply state “you should no do this”, but include the options you think might be better. Some tools allow you to provide suggestions which the author can than include without having to copy/paste things, this greatly helps your colleague / team member.
• Communicate. The author might not agree with your stance, discussions happen and should happen. In the end your discussions will improve the code and you’ll probably notice these discussions in time tend to shift to prior to merging, for instance during refinements.
• Consider timing. This is sometimes hard. You probably have a feature branch you’re working on. But your team member might be waiting for someone to review their work. As a team, discuss how you want to handle these situations to prevent frustration from building.
• Take it seriously. As discussed above, take your review task seriously. Take the time you need, don’t rush yourself. If there’s parts you do not understand, ask for help. Have the author explain things when needed. Remember that YOU are the quality guard here.

Receiving feedback

On the other end of feedback, the author wil receive comments on their work. This too is not always easy.

• Be open for criticism. Not all of your code is awesome, no matter who you are or what role you have. Like a book, you write your code in a personal style which others might not like or rather see differently. Accept this and be open to the opportunity to learn.
• Don’t pressure reviewers. Some changes might have priority or business pressure associated. But process agreements to safeguard quality are there for a reason and should not be thrown overboard when pressure is high. Have a professional attitude towards this.
• Also, respect the agenda of your reviewers. They’re not sitting idle waiting for you to present your PR to them, they’ve got work to do as well. If you find yourself in the situation where you’re F5-ing your PR to see if the reviews are in, you probably should revisit team agreements on this part.
• Take the review seriously as well. When there are comments, deal with them. Simply replying “won’t fix” and closing the comment is basically disrespecting the reviewer and won’t improve team maturity or vibe.
• Know your reviewers. Not all team members review in the same way and quality might differ due to things like seniority. If your branch has that one critical feature that might break everything, invite the most rigorous reviewer on your team to take a look. It will pay off.

Concluding

If you take everything above into account, reviews can prove to be very valuable. They should not feel as a “need to” but rather as “we really should do this”. Whenever reviews in your team feel like they’re adding pressure, frustration or other negativity, discuss this and find out why. Reviews should be a great way to overall improve team output, quality and maturity!