Code Reviews
What is a code review?
In short, a code review is when one engineer reviews another engineer's code. How should the reviewing engineer conduct the review?
Who?
Ideally, it should not be the same engineer every time and it should be done on a rotational basis except when the code in question is owned by another team or person. If this is the case the owner of the code should be part of the review. One person can review the code, or multiple persons depending on the risk the changes pose and knowledge-sharing concerns. However one should be mindful to not require more reviewers as is beneficial and necessary as this will waste time.
What to look for
Correctness
The reviewer should check that the code is doing what it is intended for. Check for possible edge cases that were missed. If you know other areas in the system, think about what areas might be affected, break, or could be impacted by this code change. Ask the reviewee if he has considered possible issues and tested for those issues. The reviewer should also test to make sure, although this will depend on time constraints.
Performance
Are then any performance optimisations that could have been made? Be careful though not to nitpick at these things and be careful not to wail loudly about performance being neglected if you are not sure the code under consideration has an issue. If you can see a quick win though, suggest it and move on.
Readability
How readable is the code? When an engineer moves to the intermediate skill level he often starts thinking it would be great to show others how smart he is by squeezing as much code as possible into the least possible amount of lines. The less the better, even at the cost of readability. This should be avoided. Don't expand the amount of code you write needlessly, strive to write the least possible amount of code that still maintains the maximum amount of readability.
Maintainability
Is the code structured in such a way that it can be maintained with relative ease? Be careful to take into account time constraints the reviewee might be working under and also be careful not to overengineer.
Architectural conformance
Does the code fit into the team's architectural pattern and rules? If classes or other objects were added, what function do they perform? Do they perform their responsibilities according to the team's agreed-on standard? (This assumes there is a standard. If there is, I highly recommend that it should be documented.)
Attitude
Creating tension and nitpicking
Code reviews can be a great opportunity to share knowledge, level up juniors and maintain code quality. However, if it is done wrong it can be infuriating to the reviewee and create tension in the team. Therefore I will offer a couple of considerations. First, do not go into a code review with a vendetta and with your goal being to urinate all over your colleague's hard work. Your goal should be to offer friendly constuctive comments. Second, do not enforce your own personal style on others. If something is a team standard, point to the standard and ask that an update be made. Thrird, do not nitpick. There are some issues that are important and others that are not. If you do want to nitpick, then tell the reviewee that the fowlling comment you are about to make is just a nitpick.
Rules
Software development has very few infiolable rules. If such things exist they point to underlying laws of logic and physics. Rules are good, best practices are recognised as such for a reason, however, the exception proves the rule. Do not adhere too dogmatically to a rule and act as though it can never be violated. If you have a logical reason to deviate from the rule, then discuss it with the team and go for it.