As a reviewer you’re responsible for improving safety, enforcing broadly agreed upon standards and should use the opportunity to teach and learn from your colleagues.

🎯 The goal of code reviews

Code reviews have a few goals.

⛑ Safety

The #1 priority in a code review is to provide a layer of safety to protect things from breaking for your users.
🚔 Enforcement of broadly agreed upon standards

In the cases where there are broadly agreed upon standards that aren’t enforced by your test infrastructure we may need to enforce standards in reviews. Reviewers should never enforce standards that aren’t broadly agreed upon.
🧠 Education and context sharing

Nobody comes to a company already knowing the stack. Code reviews provide a chance for us to learn from each other.

Authors are required to get an approval from any colleague in order to merge code, but that doesn’t give reviewers a license to block reviews for just any reason.

❌ No stylistic preferences

Linters enforce style guidelines. Reviewers shouldn’t push their personal preferences in a blocking code review.
❌ No blocking tips

Offering tips for how things could be improved is great, but reviewers shouldn’t block PRs on nits.
✅Block unsafe or non-standard code

Only safety issues and broadly agreed upon standards that aren’t caught by the linter should block PRs.

⛑ Safety

The most important goal of a code review is to provide a layer of safety. The top priority of a reviewer is to look for risks that may have been missed. There are many known types of risks that folks will be looking for and many unknown risks that you’ll need to use your discretion to navigate.

🔐 Security and privacy

Any PR that introduces (or appears to introduce) a security or privacy issue should be blocked until the issue is resolved.

🐛 Bugs

Reviewers should be on the lookout for bugs where the product might not behave as intended by the author or could otherwise cause issues for users, integrity of our data, etc..

🧨 Traps for other engineers

If a method is called renameUser but it actually deletes the user we could expect future engineers to get confused and will be at risk of introducing bugs.

🕸 System failures

It’s not always easy to spot changes that could trigger cascading failures or instability of our systems. Reviewers should be on the lookout for how unexpected circumstances might impact the broader network.