ferroman@systems: ~ — fish 3.7
← ~/blog

Make Code Review Useful Again

Code Review has commonly used technique and it’s definitely one of the must-have processes to keep a high quality of code. But quite often it becomes a formal process or people experience different issues using it. So…

Code Review has commonly used technique and it’s definitely one of the must-have processes to keep a high quality of code. But quite often it becomes a formal process or people experience different issues using it. So let’s take a look at what is Code Review and what good and bad parts we can get using this technique.

WHY

Code Review is used for different reasons regarding process specifics — for example, in open source development, it is the only way to include contributed code from the third-party developers. But in a team (especially a small one) the must-have review is not necessary. But they still need it. So why people use Code Review in one team:

  1. Catch logic errors.
  2. Catch missed parts (not updated documentation, missed tests etc.)
  3. Catch missed requirements. Sometimes engineer can accidentally overlook some of the implemented task requirements.
  4. View from the outside. Someone from outside can see issues in code which developer can miss because of ‘bogged down in code’
  5. Check code quality.
  6. Knowledge sharing.
  7. Education. Doing the Code Review developers educating each other.
  8. Make sure implementation complies to coding styles, standards, etc.

GOOD PARTS

Using Code Review on a regular basis keep developers knowledge synchronized between each other and make the members inside a team replaceable, more universal, which is good for performance and keep “bus factor” number for your project as high as possible. And it forces developers to use best practices in their work — just because someone will see it, and such lazy as I am can’t just skip something because of laziness or distraction.

BAD PARTS

Unfortunately, Code Review takes time. In real life everyone is busy — so code reviews quickly became a formal process, which makes it pointless. This happens because of a few reasons:

  1. Reviewers should understand the task context, which is not easy. There can be a lot of context details — legacy code specifics, business requirements, environment-specific details etc.
  2. Reviewers should mentally switch from their own tasks.
  3. Point out other people’s mistakes psychologically unpleasant. Reviewers should be careful and polite to avoid an aggressive-defensive reaction from other developers, but we’re all human beings, aren’t we? Sometimes it’s not easy to explain your point of view in writing, and it takes a lot of time.
  4. Lack of time for task implementation. Usually, code review happens after full implementation, in the middle of the iteration, or, most likely, closer to the end of the iteration. So, usually, there is no time for any non-trivial changes (which should be reviewed too). And it’s not always obvious which changes have to be done immediately, and which can be postponed.

LET’S MAKE IT BETTER

Use checklists

Anyone participating in the code review should have a checklist for what should be checked or described. It should be short and simple. For example, to publish the review:

  1. Check task requirements are satisfied
  2. List of requirements…
  3. Cover code with tests
  4. Update media files
  5. Check that all new files are committed
  6. Update README
  7. Make sure that build is green
  8. Write a description for the review

Prepare pull request/code for a review

Do not fear. Engineers born brave, so dont let fear stop you from showing your work.

Any advice or found issue will make you better.

Always describe what you did, context, and how you did it in general. Try to be short, but always explain non-oblivious choices.

It’s good to use automatic lint programs, or even autoformatters, to check code conventions, so you will not have “use 2 spaces instead of 4”-like comments in the review.

Create more than one review for the task (especially for tasks that are going to be done at the end of implementation). Create some MVP of the feature, if possible, and send it to the reviewer ASAP. This allows to see major issues, not at the end of an iteration, and give you more time to find the better way to implement it.

Ask for advice. Two heads are better than one, right? If there is something wrong, or you stuck with the one solution but you feel that there should be something better — ask for the outside view.

Ask for a review of big code at the end of the day — this will guarantee you do not interrupt someone in the middle of the process and give them more time for review — which is good. Most likely, reviewers will do the review the next day morning, which is also good for you — the clear mind can see more.

Reviewers

Do not offer solutions at the first place.

Show problems, but don’t tell how to solve them, so a developer will have a chance to make his own decisions. People don’t like to follow orders, they enjoy to find their own way. Remember, your solution can be not so good as you think — you don’t have the full context, so there can be details you don’t know. Anyway, usually, people ask for advice.

Do the reviews first in the morning , when your mind is still clear. This will give another developer more time to fix the code, and you will not break your mind flow.

Not necessarily always find something. Sometimes things are good enough. Of course, there is always room for improvements, but let’s do things well, then do them better.

Use the ‘presence’ checklist. Just to make sure that everything is there — unit tests, updated documentation, no requirements skipped, resources updated etc.

Do review from general to details. Try to understand what should be implemented and how it was done in general. This makes your mind construct your own solution, so when you will see details, you will have the right questions to ask.

Make sure you really understand what you are looking at. Ask the author to help you with this understanding. Sometimes the best way to do this — review this code in pair with the author or another reviewer.

Use Fowler’s ‘code smells’ checklist. Using this checklist helps to quickly see the issues in the code and make your review more concrete — It’s much easier to explain what wrong if everyone uses the same definition and understanding what, actually, is “wrong” and why.

Code review is a great technique that makes team competency grow, and grow fast, and it’s very important to do it on a regular basis.

Make Code Review Useful Again was originally published in SavvyClutch Engineering Club on Medium, where people are continuing the conversation by highlighting and responding to this story.

Originally published on Medium.