Front-end Code Review – Our Expectations

This is a quick post that sets out expectations when we are reviewing code. By defining our base level for front-end code, we can make sure everyone on the team is working towards the same goal.

Code reviews are vital in ensuring the required quality of output is met. At Rareloop it’s important that all of our code looks like it was written by a single developer. While we have tools in place (like linting) to enforce rules across the team, code reviews are still massively important; especially when introducing somebody new to the team.

This post aims to set the bar for what we expect during a front-end code review. To add some context, we build all our front-end in Primer – an open-source pattern library.

We like to keep code reviews informal and collaborative, that’s why we opt for the over-the-shoulder approach. It’s lightweight and encourages a collaborative environment.

Rough structure

Somebody will sit with you while you demo what you’ve been working on. It’s important to articulate any decisions you made, areas you think could be weak or need improvement or anything you’re proud of (jazz hands ?).

Working on a project for a long period of time can cause snow-blindness, so a fresh pair of eyes can offer invaluable insight – catching the bits you’ve missed or forgotten or talking through a problem to find a better solution. The outcome being a better, more resilient front-end.

When building a component, show how it works responsively and highlight any compromises you’ve made (e.g any progressive enhancements).

Our Base Level

When a component is finished, it must meet our base level requirements… otherwise it’s not finished. The following do not need to be 100% met for a code review, as long as you’re aware and can articulate areas that still need work (e.g. I know the spacing is off):

  • These should match the desktop design. Mobile should have sensible adjustments made: fonts (size, weight, family, line height), colours, spacing
  • Components shouldn’t ‘bleed out’ (see BEM Guidelines – Margin Bleeding)
  • Correct use of BEM
  • Built mobile first
  • Built core first, then progressively enhance
  • Data driven by json, not in markup
  • Hover states
  • Use of semantic HTML

You should always build with these in mind, but it doesn’t matter if you miss. There’ll be someone in the team to catch it.

By everyone on the team working towards the same base level, we introduce predictability which helps breed resilient websites.