This repository has been archived by the owner on Dec 11, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 974
Reviewer guideline
Brian Clifton edited this page Feb 6, 2017
·
6 revisions
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.
- Make sure we want the change; just because there is a PR doesn't mean it's a feature we should be doing.
- Make sure it passes existing tests (check travis-ci logs).
- 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.
- Try the change out yourself if it is testable, this helps make it easier for understanding the code as well.
- Make sure things are labeled correctly and make sure to add the milestone the PR is being merged into.
- Front end: If styles are being added, make sure it is using Aphrodite. For more information, please see our style guidelines.
- Make sure the issue itself has the Test plan filled out before accepting
insecurity test
Vertical Side Tabs Tab Suspender