Code Review Policy.

I was recently asked to share the code review policy I maintained when working with Medium Engineering and I thought it could be useful to others.

Rules

1. All code — with the below exceptions — should be reviewed by at least one other engineer before it is merged into the main branch of a repository.
2. Code reviews should be responded to as promptly as possible.

Exceptions:

• Config changes that only affect development and staging environments do not require a code review.
• Changes that are purely lint fixes do not require a code review.
• Copy changes should be reviewed by a designer from the relevant feature team. No engineering review required.
• If a CARETAKERS or CODEOWNERS file exists in a file’s hierarchy, then one of the listed members should also review the change.

Note: Even changes that are exempted from a code review should be made as a pull-request that can then be immediately merged.

Roles & Responsibilities

Code reviews perform multiple functions: to maintain the quality of the codebase; to ensure consistency; to spread knowledge.

As the code submitter, you are expected to:

1. Own the process and push for timely reviews
2. Shepherd the change safely through to production
3. Scope the change to a reasonable level of complexity and functionality — prefer small incremental changes over ship-the-world updates
4. Ensure the change is backwards/forwards compatible and/or there is a clean revert path
5. Ensure the change is suitably tested, documented, and instrumented
6. Ensure that security requirements and top OWASP issues have been considered

As a code reviewer, you are expected to:

1. Understand the intent of the code being reviewed
2. Understand the code in the broader context of the application
3. Identify potential bugs and performance issues with the code
4. Enforce design principles and coding style-guides
5. Ensure the change is unit testable and appropriately tested
6. Respond promptly

Choosing a reviewer

When choosing a reviewer, identify someone who is already familiar with the code you are changing. Do not add a laundry-list of names to the review, add the minimal set of people who are qualified to cover all facets of the change (e.g. backend, frontend, models, feature specific).

The code review process

One of the main limitations of the GitHub review process is knowing whose turn it is. In order to avoid reviews stagnating we should be explicit about when a step has finished.

• As a reviewer, when you have finished making comments let the submitter know you are done with your pass and are expecting followups.
  “Ok, @majelbstoat, address those comments and I’ll take another look”
• As a submitter, let your reviewers know when you have addressed all the comments and pushed your changes:
  “Hey @nicks, addressed your comments, how does this look?”
• It is the responsibility of the submitter to keep the review on track and make sure it gets merged. Don’t hesitate to ping the review thread if you are waiting for a long time:
  “ping @dpup, this is ready for another look”
• As a reviewer, if your comments are simple or superficial make sure to let the submitter know they can merge without needing another round:
  “LGTM after those comments have been resolved”

Tools

Sample GitHub Pull Request Template

At Range we used a Pull Request Template to streamline the flow and help us remember what to include.

Here’s a slightly adapted version:

#### What's this PR do?

#### Where should the reviewer start?

#### Any background context you want to provide?

#### What are the relevant tickets?

#### Screenshots (if appropriate)

#### What gif best describes this PR or how it makes you feel?

#### Definition of Done:

##### Code reviewers, by giving a LGTM you attest that:
- [ ] Commits are adequately tested
- [ ] Code is easy to understand and conforms to style guides
- [ ] Incomplete code is marked with TODOs
- [ ] Code is suitably instrumented with logging and metrics
- [ ] [Security Requirements](insert-link) have been considered
- [ ] [OWASP Top 10](https://owasp.org/Top10/A00_2021_Introduction/) have been considered

##### For terraform changes:
- [ ] Change should be applied to staging prior to code review
- [ ] Change should be applied to production after approval and before merge
- [ ] Change should be backwards and forwards compatible
- [ ] Network and data flow diagrams (`docs/diagrams/`) are up-to-date

git-workflow

A bunch of Medium alumni use my git-workflow script for branch management and initiating pull requests from the command line.

$ git start new-feature
~ HACK HACK HACK ~
$ git cam 'New feature'
$ git sync
$ git pull-request
$ git switch bug-fix