Code Reviews — more than a thumbs up

Ah, it’s Tuesday morning and you open your laptop. You see a new Jira notification indicating that you’ve been assigned as the reviewer for ticket number 38 (completely random — actually, 38 is my favorite number if that counts for something).

Anyway, ticket 38 is sitting in your Code Review lane and it’s about time to go check it out. You see a linked pull request (which I’ll be referring to as a “PR” from now on) and open it up in Bitbucket. For those who don’t know, Bitbucket is the equivalent of GitHub, where code repositories are hosted and managed remotely. You can learn more about it here.

There are three key stages of effectively reviewing a PR:

1. Know what you’re working with. The need-to-knows of ticket 38 —

Requirements. This is a sample of some questions you should ask yourself when you’re reviewing ticket 38, whether it’s a bug/defect, or story/feature.

I know, I know … but I had to throw some code in here for good measure!

const questionsPerType = { 
  bug: [
    'what's the problem?',
    'how could I recreate the problem?',
    'what is the expected outcome?',
    'can I confirm that the acceptance criteria is met?',
    'which tests need to be updated?'
  ],
  story: [
    'what is the expected outcome?',
    'is there a design to reference?',
    'what are some edge cases to consider?',
    'how does this feature fit in with the rest of the codebase?',
    'what kinds of tests need to be created?'
  ]
};
const startReview = (ticketType) => {
  questionsPerType[ticketType].forEach(question => console.log(some answer));
};

If any of this is unclear, ASK the developer before moving forward!

Once you can confidently say you understand ticket 38’s requirements, move on to testing that the work accomplishes the goal. Ideally, your teammate has left you instructions for this. If not, request them. You want to ensure you’re testing the same way they are, and that you’re both speaking the same language.

startReview('bug');

2. Verify that the expected outcome of ticket 38 is implemented —

Let’s say that looks like pulling the branch and viewing changes locally in the browser… make sure that you are confirming the expected outcome, considering any edge cases and/or side effects of this implementation, etc. In the case of a bug, try to recreate the issue and confirm that it’s no longer an issue.

3. Analyze the code that got us here —

Now it’s time to check out the code. Great, we got it working, but how did we get it working?

• Ask questions. I bet you can learn something new from every PR you review. Ask for an explanation of the approach. Ask for clarification on one specific line of code that is unfamiliar to you. Ask away; your questions will not only give you a better understanding of how your teammate thinks, but they will also give the developer the chance to articulate their thoughts and decisions. A win-win in my book.
• Give Praise. And I’m not just talking about the thumbs up that this article’s title alludes to — point out something very specific that you think is clever, readable, efficient, …[insert positive programming words here].
• Offer Suggestions, humbly. See a function that can be made more DRY (Don’t Repeat Yourself)? See an opportunity to make something more dynamic and, therefore, scalable for future cases? See some unused code that should be removed? See the potential for future technical debt? Say it kindly. Share your ideas. Link an article that provides a more thorough explanation of what it is you’re suggesting. Better yet, go tag someone else in your comment so you can spread the learning.
• Cross-reference a checklist if there is one. Has the developer provided all of the necessary information to make this code review as simple as possible for you? Have you analyzed the PR with a keen eye? Have you ensured this new or modified code hasn’t introduced new errors or warnings? Have the appropriate tests been created or updated so that we can catch any functionality that breaks with future implementation? (I guarantee the test you write will take less time than the time it’d take to hunt down your bug).

Wow, what a powerful learning experience for all involved!

If we rewind to a little while ago and instead, you’d decided to breeze through the PR, you’d have lost out on an opportunity to grow your team’s skills AND the ability to ensure the project consists of quality code.

Leave your “thumbs up” in the past and start engaging with your code review process!