Code Reviews 5: Good articles on code reviews

Let me start by saying "good" is subjective and it's based on my opinions of how code review must be done, if we are working together.

This post is a collection of resources I've come across on code reviews.

Good code review contributions

These articles align, by and large, with how I feel code reviews should be done correctly

1. Preparing your code Review
2. Code Review Tips
3. How to be a better code reviewee
4. How to be a better code reviewer
5. Code Review Guidelines for Humans

Code review resources

These are generic inputs that you will need for code reviews

1. A Style guide for our source code. Googles Java Style Guide or the original Java Style guide, might not be the best. Write one or pick one, but once you choose for a particular product, stick consistently to it.

Code review tools

Effective tooling really helps enforce a proper code review flow and keep the quality of the review high. Here are a selection I've come across.

1. Gerrit. This is the tool I'm currently using and I'm happy with it.
2. Review4Eclipse. Another tool I've used for reviews. It's a lot better than having no code review tool.
3. Jet Brains Upsource. From the makers of IntelliJ. This is next on my list to try, I hope it's as good as IntelliJ!
4. Smart Bear Collaborator. I haven't used this, but it looks interesting. It appears to be the most feature rich. One I would like to try out.

Code Reviews 4: Three Principles for doing effective Code Reviews

In order to do effective code reviews I propose the following three Principles of Code Review that every professional developer in your team must sign up to.

Code Review Principles for Developers

1. Nothing gets merged to the master branch without the approval of a code review. Ever. Treat master with respect.


2. After two¹ approvals have been obtained, the author merges work to master.


3. When I take on a fellow engineers' code review I must progress the review immediately as the most urgent item on my list.

Reasoning the principles

Professionals operate by certain principles and they apply them consistency and fairly. Let's explore some of the reasons behind why each of these principles are important.

1. Nothing gets merged to the master branch without the approval of a code review. Ever. Treat master with respect.
1. I am human and I make mistakes. I require the validation by my peers on what I have done.
2. As a professional, I have a duty to educate others. A peer review is one valuable way to share knowledge.
2. After two* approvals have been obtained, the author merges work to master.
1. The author should be responsible for the full completion of their own tasks.
2. The author asks for and appoints people to complete the review. At least one should be senior and experienced. Those appointed people stay with the review until it is complete i.e. merged to master or abandoned
3. All actions between the authors and reviewers are closed pragmatically with necessary rework inspected and complete. Code is never merged with any outstanding code review comments. All review comments are either implemented or discussed with reviewer and agreement reached as to why comment is not acted on. The decision and the why should be recorded in the source code as a comment.
4. Sending the review to the entire group leads to confusion as to who is doing the code review. IF everyone is responsible, then no one is responsible. Be clear who exactly is doing the review and appoint them to the review.
3. When I take on a fellow engineers' code review I must progress the review immediately as the most urgent item on my list.
1. This is context switching, but we value “done” tasks over “work in progress”. To help flow of tasks in my team, we realise that people at code review are closer to “done” than I am – so unblocking them is my top priority and the context switch is more valuable.
2. If I have more urgent items to tend to, I do not take on the code review.
3. Some one must take the code review, so the team might need to discuss priorities.
4. If I do this review now, when my turn comes I can count on my team mates to drop their tasks in progress so that my code review progresses rapidly.

---

1. I recommend two. You may choose whatever number suits your project. But when you choose – stick to it consistently. This is so we are fair to every developer working on the product. Picking too small a number, means we don't get enough feedback. Picking too large a number requires interrupting more tasks in the team to progress the task in review.

---

Thanks for contributions from: Rachel O'Toole, Shane McCarron, David Hatton

---