forked from CesiumGS/cesium
-
Notifications
You must be signed in to change notification settings - Fork 2
Code Review Tips
pjcozzi edited this page Jul 9, 2012
·
12 revisions
In progress...
All of our code is peer reviewed before merging a pull request into master. We review code to share knowledge; foster shared ownership; and improve code quality and consistency.
Tips
- GitHub's code review tools are awesome.
- Anyone is encouraged to review anything that interests them. However, someone familiar with the changed code should ultimately merge the pull request.
- Some changes can be reviewed by reading the code alone; others require running the code, examples, and tests.
- Most pull requests require additional commits, often minor but sometimes major, before being merged.
- Comments are about code, not the person who wrote the code. Don't be offended by a reviewer's comments and don't aim to offend when commenting. We all have the same vision and want the same thing - to be awesome.
- Be concise. Make every word tell. Less reading means more coding.
- Provide motivation. Unless it is obvious, don't just suggest a change. Suggest why the change should be made.
- See the forest through the trees. Don't just review code one line at a time. Consider the big picture and its implications.
- Bring other people into the conversation. If someone has expertise with a particular language feature or problem domain under review, invite them to help.
A pull request is a sincere request for feedback, and those reviewing the request work with the developer to get the code merged.
Resources