THIS IS A TEST INSTANCE. ALL YOUR CHANGES WILL BE LOST!!!!
| Note | ||
|---|---|---|
| ||
This document is subject to change without notice. |
What’s a code review for?
...
For the reviewer it is an opportunity to learn not only about the code, but about the reviewee. Which kinds of quality does he value? What skills does he bring to the table? What kinds of problems can occur with code reviews?
What kinds of problems can occur with code reviews?
By nature a code review is an asymmetrical exchange. The code reviewee offers his work product, the reviewer offers a third and fourth eyeball. This is the main source of problems in a code review. The reviewer, out of respect for what the reviewee has done, may hesitate to point out real problems in the code. The reviewee, out of pride in his accomplishment, may find it difficult to accept legitimate well-meaning criticism. Or worse, the reviewer, out of envy that he didn’t get to make the change, may complain about features of the code that are matters of taste. The reviewee, out of insecurity, may allow himself to be pressured into making changes he doesn’t believe in.
...
This list of questions should help both the reviewer and the reviewee keep a code review on track. You’ll notice that at no point do we state what the answer to a question should be. That is not because we don’t have an opinion, and most of the time that will be obvious. However, it is not our code review; it’s yours. The code reviewer and the code reviewee will decide together what the correct answers to these questions are. Just make sure you’re not avoiding any of these questions because you’re afraid you’ll fight over the right answer.
Correctness
- Does it make sense to fix this bug or add this feature?
- Does the code change solve the problem it is intended to solve?
- Does the code change degrade the performance of the code?
- Does the code change cause any undesirable program behavior?
- Does the code compile?
- Is it free of warnings?
...
Do the changes effect deployment of the system?
Are the deployment changes documented in a place that the effected users can find?
Do the changes demand no more configuration from user users than that information which the software absolutely cannot determine automatically?
- Are configuration defaults reasonable?