Reviewer guideline

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. For further information regarding Aphrodite or if you're refactoring styles, please check the refactoring styles wiki
7. Make sure the issue itself has the Test plan filled out before accepting. Preferably, post the test plan to the top of the issue for QA visibility. If there is automated coverage, list the test with grep statement needed to run the test. If there is sufficient coverage, the issue can be labelled no-qa-needed.
8. Follow the pull request approval process.