Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Reviewer guideline

Brian Clifton edited this page Jan 17, 2017 · 6 revisions

Becoming a reviewer

Reviewers can be employees or contributor members that have several PRs merged, worked with code in this repo a lot, and are vouched by an existing owner of that code. Being an employee does not make you a reviewer.

Reviewers can be scoped to only certain parts of the code.

Sometimes a user that is not a reviewer can be used if it is a very localized change related to some small bug for a recently landed change that the reviewer made.

Guidelines for reviewing

  1. Make sure we want the change; just because there is a PR doesn't mean it's a feature we should be doing.
  2. Make sure it passes existing tests (check travis-ci logs).
  3. Make sure it has new tests if possible, prefer unit tests when possible. We'd rather have features land slower than have un-tested features.
  4. Try the change out yourself if it is testable, this helps make it easier for understanding the code as well.
  5. Make sure things are labeled correctly and make sure to add the milestone the PR is being merged into.
  6. Front end: If styles are being added, make sure it is using Aphrodite. For more information, please see our style guidelines.