So you have a PR: A Code Review Checklist

Reader, be it is open-source or in-house code, front-end or back-end code, the pull request (PR) model is widely used in web development. But, before a PR is merged, it should be reviewed by peers, and the code modified by the developer as needed, in an iterative process. Ideally, we want to spend quality time reviewing each modified line so that we don’t build tech debt when code is added or modified.

But reader, you know as well as I that with pressing deadlines, sometimes we simply don’t have enough bandwidth for code reviews. And when we do have time, we may not be doing as good a job as we could because we don’t have a clear set of team coding guidelines or we may not know what we should look for. So, below find some suggestions about how code review can be done, with specific front-end things to focus on.

  1. Thorough and meaningful code reviews take time. We need to scope for code review just like any other task. For that to happen, everyone must believe in its purpose and importance — that means management and product managers must actively encourage code reviews and give developers the necessary time.
  2. The team has agreed on naming convention, styling, and coding standards, whether it is HTML, CSS, or JS. Styling standards give the code a consistent and professional look. A naming convention makes it easier to understand, search, build, and maintain. Coding standards make the code cleaner and easier to read, build, maintain, and debug. There are many popular standards out there. It is less important which one you choose, but it is crucial that the entire team follows one standard. If a rule is controversial within the team, discuss and decide, then change the standard and apply the new rule across the entire code base. Consistency is key.
  3. The team has agreed on what to review — from spaces and camelCase, to where business logic should be, to whether the new code is violating established architecture.

When Reviewing HTML:

When Reviewing CSS:

When Reviewing JS:

A few notes on jQuery:

Browser support

One difficulty in front-end development is getting a web app to work well across devices, operating systems, and browsers. With the large number of device, operating system, and browser combinations in the wild, including the old and the native, working well everywhere is an impossible task for developers. The reasonable approach is to get your app’s traffic breakdown and test the most popular combinations during development.

To review layout and browser quirks, an in-person code review is the most effective as the reviewer can play with the feature/fix while reviewing the code. (A very simple exercise, which is also very effective in finding layout bugs, is resizing the browser window.) If an in-person review is not feasible, attach screenshots of the app to the PR, showing the layouts in different forms: