A guide for reviewing code and having your code reviewed.

Code review is an important part of a team's development process. It helps to:

• disseminate knowledge about the codebase/technology/techniques across teams
• catch poor architectural decisions and bugs
• provide opportunities to educate junior engineers

• Accept that many programming decisions are opinions. Discuss tradeoffs, which you prefer, and reach a resolution quickly.
• Ask questions; don't make demands. ("What do you think about naming this :user_id?")
• Ask for clarification. ("I didn't understand. Can you clarify?")
• Avoid selective ownership of code. ("mine", "not mine", "yours")
• Avoid using terms that could be seen as referring to personal traits. ("dumb", "stupid"). Assume everyone is attractive, intelligent, and well-meaning.
• Be explicit. Remember people don't always understand your intentions online.
• Be humble. ("I'm not sure - let's look it up.")
• Don't use hyperbole. ("always", "never", "endlessly", "nothing")
• Don't use sarcasm.
• Talk in person if there are too many "I didn't understand" or "Alternative solution:" comments. Post a follow-up comment summarizing offline discussion.

• Be grateful for the reviewer's suggestions. ("Good call. I'll make that change.")
• Don't take it personally. The review is of the code, not you.
• Explain why the code exists. ("It's like that because of these reasons. Would it be more clear if I rename this class/file/method/variable?")
• Seek to understand the reviewer's perspective.
• Try to respond to every comment.
• Wait to merge the branch until Continuous Integration tells you the test suite is green in the branch.
• Merge once you feel confident in the code and its impact on the project.
• Provide context about what the change is and why is it important - Write informative commit messages[4] and pull request descriptions.
• Use the pull request description template[3].

Understand why the code is necessary (bug, user experience, refactoring). Then:

• Communicate which ideas you feel strongly about and those you don't.
• Identify ways to simplify the code while still solving the problem.
• If discussions turn too philosophical or academic, move the discussion offline. In the meantime, let the author make the final decision on alternative implementations.
• Offer alternative implementations, but assume the author already considered them. ("What do you think about using a custom validator here?")
• Seek to understand the author's perspective.
• Sign off on the pull request with a 👍 or "Ready to merge" comment.

Some suggestions of things to look for when reviewing code[7]:

• Is all the code easily understood?
• Does it conform to the agreed coding conventions?
• Is there any redundant or duplicate code? Is the code as modular as possible?
• Could the design of the solution be improved?
• Is the code testable? i.e. don’t add too many or hide dependencies, unable to initialize objects, test frameworks can use methods etc.
• Do tests exist and are they comprehensive?
• Does the code work? Does it perform its intended function, the logic is correct etc.

Links from places on the Internet that helped write this:

• [1] Talk - Implementing a Strong Code-Review culture - the talk that inspired this.
• [2] Thoughtbot's Guide for Code-Review - straight to the point guidelines on code review. A lot of the content was taken from there.
• [3] Pull request templates make code review easier - introduction to pull request templates.
• [4] some articles on how to write good commit messages - a b, c
• [5] Am I My Code? - this article explores some strategies for giving feedback when reviewing code.
• [6] Code review at the Dropbox iOS team - the article goes over their whole process of review. It has some interesting ideas.
• [7] Code Review Checklist - from Fog Creek Software