Henry Chong

Code Review Principles Workshop

Someone using a MacBook to code while video calling a colleague.

📷: Photo by ThisIsEngineering from Pexels

We ran a half day event for a relatively new team that I'm managing, and one of the sessions I walked our group through was a Code Review Principles workshop.

Like any other form of communication, everyone has varying preferences when it comes to reviewing code, and we wanted to build a shared understanding and alignment on this topic. Whenever a group of engineers are coming together for the first time and expecting to work closely, I'd recommend running a similar exercise.

We brought our ideas into a shared Figjam, had an open discussion about code review (what we think works, what doesn't) and then came up with some principles together.

Our team found the process and discussion useful, and here's what we ended up with:

Author Responsibilities

  • Stop starting, start finishing
    • Before you start a new bit of code, see if you can help finish any existing code reviews
  • Keep PRs small
    • PRs you ship should keep the codebase in a working state, especially production
  • Provide evidence that your PR does what it says
    • This can be tailored to the contents of the PR, it could be a screenshot, video, or a unit test as part of the PR
    • It's also useful to provide test accounts, explanations, links to documentation and any other way to help the reviewer kick the tyres if they choose to
  • Move the PR forward
    • Proactively follow-up, chase and take any actions needed to maintain momentum

Reviewer Responsibilities

  • Provide a review outcome within 24 hours
    • There must be a clear accept or reject outcome
    • If alignment can't be actively reached within this timeframe, escalate or find a way to more progress
  • Debate productively
    • Remember we're all on the same team
    • Keep it about facts, logic and the topic
    • Don't make it personal
    • Be intellectually humble
  • Clearly indicate non-blocking nit comments
    • E.g using the “nit:” prefix
    • a nit informs the author there could be a better way for this to happen, but the reviewer is not expecting it for this PR
  • Don't be afraid to jump onto a cuddle
    • Okay, we mean huddle, or video call
    • tone and importance can be hard to gauge with comments
    • complex topics and consensus is easier in real-time

Do you relate to these? How does your team do code review differently?

(Special mention to this summary of a Harvard Business Review article on How to Debate Productively that I came across on LinkedIn)

Legal & Privacy Notice