Goal: identity significant problems so they can be fixed before deployment.
Ensure the code is:
- Bug free (for bugs with a significant chance of happening)
- Generally meets the goals of the change
- Has appropriate automated testing
- Doesn’t cause unexpected problems
- Doesn’t decrease security, scalability, robustness, etc
- Maintainable and understandable by future developers
- Code technique generally fits with the rest of the code in the repository.
- No unreasonable risks
- Answers important “why” questions. Code inspection often easily answers “What” but look for important unanswered “Why” questions
Reviewers aren’t expected to:
- Find all bugs
- Enforce style (formatting, way of doing things, etc) as long as it fits reasonably with the rest of the codebase
Suggestions:
It is good to make optional suggestions as long as it is clear that they are optional and don’t affect approval.
Posts
