Table of Contents
Read the Documents
Read the documents first! I mean it! Usually at least a README.md file would be available with any repo nowadays. Get yourself familiar with the code base and know what it does, how it does.
Know the Context
Get to know the context! This is crucial! I mean it! Many reviewers conduct their reviews line-by-line knowing nothing about the context. If you don’t know the context your review might not only be useless but also time consuming to reply to.
Do not Make Assumptions
Do not make assumptions! Do not submit comments or code snippets based on your assumptions if you don’t know the context or what the code exactly meant to do.
Do not Submit Code Snippets
Do not submit un-tested code snippets! Unless the snippet you want to submit is better than the existing one, you tested it and made sure that it works as expected with the changes in the PR.
Do not Ask Questions
Do not ask questions in the comments regarding why the developer has chosen that way to implement something. Do not ask questions to understand what the code does to request changes. Read the documents. Use other avenues to learn about it. Code reviews are for code reviews!
Leave Personal Preferences Out
Do not comment on something, which is not outlawed by the established convention. Comment on coding style only if the code violates established coding convention/standards. Leave your personal preferences out when you want to recommend changes. Remember, some people like tea more than a cup of coffee. Catch my drift?
Nitpicking
Nitpicking can be allowed if it’s polite, meaningful and improves the code quality. Though, let me tell you once more, leave your personal preferences out! No “I would name this that way” if it’s named by the book. Don’t know about it? Read the book!
Use a Convention and Tools
Use a strict convention and tools to enforce it. I mean the tools that don’t make your code hard to read. Many basic aspects of a “code review” can easily be automated by the tools. So there would be less comments in the reviews to begin with. Who wouldn’t want that?
Architectural Changes
Requests to change the architecture of a piece of software indicate a dysfunctioning team. Why? Because architectural decisions should be made before starting to code. You better take a look at how your team communicates and work together. Find ways to improve it.
Do not Comment! I mean it!
Do not comment on something if you can’t produce a better version of it or know better about it. Do not comment anything based on your assumptions if you are not sure, even if you are expert in that context.
Be Constructive!
Do I even need to say that? Of course not! Keep producing clean code!
