The 7 Code Review Manners

1. No Ego

That is — if you want the opportunity to learn from others and grow.

2. All your code are belong to us

The code that is being reviewed belongs to, and is a product of, a team. Instead of asking “Wouldn’t you prefer x y z because of 1 2 3?” you can ask “Wouldn’t we prefer x y z because of 1 2 3?”.

3. Style is subjective. Linter is objective.

Instead of repeated, subjective, comments about style — leave it to the linter to be the source of truth.
See people writing code that you really really don’t like (but works)? Great! After convincing everyone that your style is preferable — add a task to improve the linter and reflect that.

4. Readability > Performance (unless proven otherwise)

There is no need to optimize idiomatic code that did not fail. Be careful where you trade-off readability for performance, or you will be left with a very performant but impossible to maintain code.

5. Comments > Change Requests

Managed git services (e.g. GitHub) distinguish Comments from Change Requests. Know the difference between something that is objectively wrong and broken and something that you, subjectively, don’t like. The former is the only reason for a change request. You merely don’t like something? Add a comment. And approve the PR.

6. Praise good code

“I really like this module, its clean and very well tested”. Yes. This is also a part of a good code review. Imagine only hearing the bad things about your code all the time. Wouldn’t that be just terrible?

7. Do not breach scope

This is something I see a lot and I credit git for it. Reviewers tend to review changed lines with already-bad code along with changes the coder did.
While already-bad code should be dealt with, its not practical to involve them in the pull request that merely touched the same lines with them. Even if they’re very small.

Cleaning up these small things should be done in a separate PR. You can even add routine tasks for them and list them as subtasks when you run into them in PRs. Most of the times these housekeeping PRs involve no logic changes, are easily reviewed, merged and deployed — and most importantly — do not block a feature.

Bonus for pros

Alias your `git blame` to `git credit`

git config --global alias.credit blame

Because we can all use some more appreciation and less finger-pointing 💜.

--

--

--

coding the tech debt of tomorrow

Love podcasts or audiobooks? Learn on the go with our new app.

Recommended from Medium

Building a Java REST API

$SIDUS - new token listing on Bybit!

Easy Paraphrasing by Back-translation

Five Common Problems that Cloud-Based Integration Helps Businesses Solve

Let’s See How the Factory Feature Used in Laravel 8

VSCode for Green Screen Developers

Azure SQL database V/S Azure SQL Virtual Machine

Get the Medium app

A button that says 'Download on the App Store', and if clicked it will lead you to the iOS App store
A button that says 'Get it on, Google Play', and if clicked it will lead you to the Google Play store
Reut Sharabani

Reut Sharabani

coding the tech debt of tomorrow

More from Medium

Embracing “Continuous Usefulness”​ in software development

How to Support Employee Growth & Professional Development | A software manager’s 2 cents.

Are you a Software Development Manager, me too, here my expectations from the role, in my humble…

5 Things I wish I knew before managing a tech team