Code Review

My thoughts on Code Review, when code review should happen and what to review during it

Code Review is the process by which another developer looks through the code you have produced. The code reviewer will act as another set of eyes on the code with a view to spotting room for improvement.

It is good to get into the habit of doing code review, much like the habit of documenting it will get easier the more you do it.

Don’t be scared of code review

When you are writing code it is incredibly easy to write something that makes perfect sense to you and no one else. Everyone has done this.

It is also really easy to miss an edge case or to haven’t considered a particular way to solve a problem.

Feedback from a code review will help everyone involved grow as developers.

What to review

If a someone requests a review of work in progress, then maybe you can review as part of a pair session. In general I like to do code reviews on Pull Requests.

Things to look for in a review

This isn’t an exhaustive list, here are some general things to look for;

  • Relevant tests have been added
  • Code that will no longer be needed post-change has been removed (not just commented out)
  • Appropriate documentation has been updated/added/removed
  • Code is DRY
  • There is no hardcoding
  • Best practice development patterns are used

Here are some specific frontend things to look for;

  • Elements have appropriate a11y
  • Elements have been marked up semantically

Here are some specific backend things to look for;

  • Best practice has been used for routing
  • Response status code for controller actions are considered

Commenting on a Pull Request

During review, if you find an issue or have a question about a particular block of code the best way to comment is in line, we use Github which makes this easier, click on the line you are reviewing and write your comment.

Approving a Pull Request

If you have found nothing you would suggest improving and think this is good to merge and deploy then comment on the Pull Request with something positive (commonly we just 👍 it).

More recently we’ve been using Github’s in-built review feature, which allows for people to mark it as having passed review without needing to comment.

Large Pull Requests

If you feel the Pull Request has too many moving parts that it is impossible to review well, you can ask the submitter to break up the PR into several smaller ones.


Recent posts View all

Ruby

Forcing a Rails database column to be not null

How you can force a table column to always have something in it with Rails

Writing Marketing

We've deleted an article's worth of unhelpful words

We've improved several pages across our site by removing words that add no value, and often detract from the article.