As the author

• Keep the reviews short, people will find time to review it a lot easier. Keeping context for a long code review is very hard which makes it take disproportionately longer.
• Include a description that explains what the code is for. Write for your audience. Explain the motivation, context, usage, link relevant tasks, and explain how you tested it. This is particularly important for things the reviewer is only vaguely familiar with. Including live preview links, screenshots, or program input/output can be useful.
• Call out explicit people to review your code whenever necessary. Avoid bystander effect.
• For very complex code, or new code that the reviewer may be unfamiliar with, it may be useful to run through the code with them in person.
• Don’t take anything personal. You are not your code. Have a growth mindset.
• Squash your code when you commit. And write concise but meaningful commit messages. Always reread your message and consider what it means taken out of context.
• Avoid unrelated changes. Especially renaming, whitespace, or minor refactoring changes. This leads to a change explosion and makes what you are doing less clear. If you want to do those things, do them separately.
• If you are fishing for a code review, do code reviews.
• You can screw yourself over by working too far ahead of your review. If you’re pending a review and want to add additional features, keep the interfacing between components clear of changes. Imagine if you had to make changes in response to the review, you don’t want to spend a lot of time remerging those changes down and refactoring the derived code. This means you should plan ahead just a little in prior code changes for fast moving projects.
• If you don’t address things immediately, clearly track them in issue tracking software if it’s a large item, or in a comment for small recommendations as appropriate. It is important to establish trust that you will follow through with the recommendations of others on your team. Team members will be inclined to comment if they feel their recommendations and input are valued.

As the reviewer

• Being a better developer will make you a better reviewer. Being thoughtful in your reviews will improve your own code. It’s a feedback loop.
• Being the gate keeper for the user experience is more important than anything else. A lot of decisions that affect the users can get made in the implementation details. Make sure you recognize when a decision is being made and surface the discussion as appropriate.
• Indicate priority of your comments to the author, a lot of people use the convention of prefixing low priority comments with “nit” for “nit pick”.
• Don’t block people, understand the difference between theoretical perfection and the approach that was actually taken. The theoretical extreme of any coding principal is almost never attainable or desirable. This is because good systems of principles create tension. Understand the tradeoff being chosen here. People repeat patterns that are already in the code. The person who breaks ground sets the tone for the levels of abstraction or ways the code is structured. If you are this person realize your responsibility. People will not likely restructure your code, you’ll have to make the improvements yourself.
• Personal preference should not trump the conventions of a particular code base. Consistency is a critical tool for helping developers make assumptions. Inconsistent code bases are extremely difficult to work with.
• Unit tests basically codify the assumptions of the developer and coverage is quite frequently used as a measure of test quality, meaning they can be subject to being artificially bloated without actually improving test quality. See Goodhart’s law. I always prefer functional tests for correctness and continued assurance as the code evolves. As such if the author is going to spend some minimal effort in testing make it an end to end, integration, or other similar functional test.
• Always fully explain the reasoning when you make suggestions. Be robust in providing links and references.
• If you have to ask for an additional explanation of something while looking at multiple related files and changes, then something is missing in the code itself. Either the code should be simplified, or a comment should be left in the code itself. Don’t settle for a response to a comment on the code review, make sure the author understands they should fix the underlying issue.
• If you see a repeated problem, don’t just fix the problem but fix the process. Why is the change commonly done wrong? Can you automate that change, or set up a process or operating procedure outlining how to properly make that change? Relying on yourself catching things all the time is not scalable.
• Lookup things often. Don’t ask the author to do the ground work for your lack of understanding. Know your search tools and how to look up things. This is as important a skill in reviewing code as it is in writing code. Especially if code is not dynamically linked in your review tooling.
• Checklists are great, as a tool, but don’t rely on them to replace common sense. They can be a great way for managing cognitive overload or complexity for critical, complex, or infrequently used code. Don’t become dependent on process, unless that process is built for change itself.
• Prioritize your scrutinization. Don’t rip apart a CLI utility that will only see internal use. Consider the goals, scope, and longevity of the code being reviewed. Suggesting massive changes to raise the bar on quality can be counterproductive for something that is intended to be a simple hacked solution to an internal problem. If you disagree with the level the problem is being solved at, and think that it is worthwhile and a good ROI to solve it at the higher level (solving the problem for more people) despite the added cost, bring this up with the author separately.
• Understand how the code will be used. Identify the scaling goals and factors of the system, and match how the code is structured to that. The most important thing to ask is: Can the code be optimized? Don’t ask for the optimization to be done. Ask for the right structuring to make it possible.

Code may abstracted such that individual calls are made to an underlying service (like S3). This underlying service is understood to be the bottleneck in the code execution, and the SLA or scaling goals of the system are possibly affected. These operations to the underlying service may lend themselves to being batched, parallelized, or removed/combined as redundant. Does the abstraction chosen lend itself to making that happen? If not, make a suggestion.

• Set aside time for reviews every day. Prioritize them the same way you would prioritize them if they were your own work. Tackle the most important functions first.
• When validating any interface, come up with 2 theoretical implementations and not just the one being implemented. The interface should work for either.