loading...
Cover image for The 3PM code review rule

The 3PM code review rule

mpermar profile image Martín Pérez ・3 min read

Recently I was giving a talk at a local engineering group about the software practices we do at our team and it did surprise me how many people came back saying they did like a lot this very little thing we do to accelerate the whole pull request / code review protocol.

In reality, the idea is super simple:

Every day at 3PM is Code Review time.

But, wait, how does that exactly work?

For us, it essentially means that at 3PM we have scheduled a fixed 1h slot in everyone's calendar. This forces everyone to stop what they are doing and to dedicate that time to peers that are waiting for valuable code review feedback. Having the time officially allocated helps people not only to remember but also to realize that code reviews are very important for a healthy delivery pipeline.

Before this practice, we had a few problems with code reviews:

  • People forgot about code reviews. Initially, we tried to insist on pushing peers to spend some time in the code reviews. To insist. It worked at first but soon both ends would forget. The reviewee would jump into some other story and forget about telling reviewers to get into action. On the other hand, the reviewers would instinctively wait to be pushed interpreting that otherwise, the pull request was not that urgent to be reviewed.

  • We do gate PRs by having two approved code reviews. Sometimes people that were too deep into their own user stories would play sneaky and wait for the other reviewer to do an initial review hoping that they can jump into reviewing much later once the initial details have been sorted. Of course, sometimes both reviewers would also try to play that game. You know how it ends.

  • As a consequence of the above points, most of the user stories were closed later in the sprint. Burndown charts had very usually the shape of a big step. But more importantly, as the sprint end approached, people would rush into trying to get their stories reviewed while at the same time they were being pushed to review other stories and rushing to fix review comments. This usually ended with code with lesser quality than expected to get into master.

  • As an engineering lead, I try to keep an eye on reviews. I always look at the critical ones and give feedback but I do defer approvals to the team. Very often I was being asked to review without anyone having looked at the stories yet. Usually, the reviewer would do this as an attempt to move things that got naturally stuck by the way we were working.

Now with the new policy in place, I believe we got people into some much better habits. These are some comments on how the team is executing the policy and some personal observations:

  • The 3PM review policy does not prevent people from doing their code reviews at any other time. People can do code reviews earlier, or even later. But at least from 3PM to 4PM they need to stop and do that work.

  • Code review time, which is one hour for us, can be used to review either alone or in a group. Now, very often the reviewee and reviewers will join in a pair reviewing session to discuss the changes. This enforces some good dynamics as before there was no fixed time for meeting and people did rarely pair.

  • If someone does not have anything to review that day then the time can be used on any other tasks.

  • Since we have established this policy we have managed to keep a much shorter queue of open PRs. Burndown charts still have steps. I will not lie. But these steps are not as steep as they used to be, especially at the end of the sprint.

  • The hour is totally arbitrary. We did choose 3PM because we thought at that time people would have spent already a good amount of work on the daily tasks and would be a good moment to do a context switch.

  • Finally, now I do have to review much fewer PRs. I can keep an eye on the critical ones without being asked to review for expediting things.

I don't think this technique is original. In fairness, I got the idea from somewhere that I swear I can't remember, we just formalized into a rule. The 3PM code review rule.

What about you? What techniques do you apply to try to expedite code reviews? Would love to learn from others.

Photo by: Sarah Pflug

Discussion

pic
Editor guide
Collapse
cristinaruth profile image
Cristina Ruth

Great read! 🙂

I implemented a similar process with my team. Daily team code reviews of any work that's in progress.

My reasoning behind it was different though:

  • Share knowledge with the entire team. Knowledge shared among all members of the team empowers the team members to help each other as needed. It also helps all members learn from each other in terms of patterns and code styles.

  • Keep review scope small. Code can change a LOT in just 1 day. So reviewing the code daily helps the team keep up with the context and the scope of the review is smaller than if you were to review a "ready" pull request.

  • Build team trust. Reviewing together and learning how to work with each other opens the door to increase team trust. High team trust = highly performant teams.

Code reviews are a great way to ensure code quality but I love to use them as a tool to build up and enable the team as well. 👍

Collapse
mpermar profile image
Martín Pérez Author

Way to go Cristina. Love your approach.

Collapse
avalander profile image
Avalander

We have a similar rule in my team. In the morning, first thing you do before starting/resuming other work is review open pull requests. That way we ensure that our pull requests will be reviewed latest next day in the morning and we don't break focus when people are working on other things.

Collapse
maksimov profile image
Stas Maksimov

Smaller teams and use of Slack is what works for us. Every single PR gets attention without any policies.

Collapse
thisiszvkh profile image
Zakiah Ismail

That's fantastic! I believe it helps speeding up the review process tremendously. How did you get everyone to be on board for the one-hour review session? Any tips?

Collapse
mpermar profile image
Martín Pérez Author

Never was an issue. I think it's because every two weeks we share team stats and it was pretty obvious that we always rushed at the very end of the sprint and needed some process around this.

One hour is the upper limit. In reality, many times people do reviews outside of that window. The official time allocation forces a notification which is the most valuable thing here. That reminder, calls people to action. Sometimes, people will spend the whole hour, if there is some design to discuss for example. Many times it is just a few minutes because you have tiny things to review and you spend the rest of hour doing something else. And other times there is nothing to do.

At the very end, the point is creating the habit.