Code Review - let's do more!
Tags: codereview, pullrequest
Published: 2019-02-24 07:17:25

---
title: Code Review - let's do more!
published: true
description: Code Review - explaining all the W questions and some best practices
tags: code review, pull request
cover_image: https://c1.staticflickr.com/3/2203/2245445147_ff54c5997d.jpg
---

Today I am going to discuss all the What/Why/Who/How questions about code review from my own experiences.

#### What?
* You wrote some code and want someone from your team to take a look before pushing it into production.
* In git workflow, developer write some code, push his/her changes and then open a pull request (PR) to let others review his PR.
* It's becoming industry standards in CI/CD pipeline, even some companies wont let you merge code without getting your code reviewed. Here you can find [dev.to](https://github.com/thepracticaldev/dev.to) all open [PRs](https://github.com/thepracticaldev/dev.to/pulls)

#### Why?
* To get feedback from peers
  * Design principles, code structures etc.
* Find potential bugs early
  * Does it do what it's supposed to do?
* Sanity check about your changes
  * Any potential typo (also covered by static analysis tools if supported)
* This is the right way to add any changes into production codebase!

#### Who?
* Anybody can review anybody's code in your team unless otherwise defined by the team.
* Active reviewer - someone who was assigned for review.
* Passive reviewer - anybody else who is interested with the underlying codebase, can ask questions/clarifications about the changes.

#### Where?
* Online - Github, Atlassian Crucible, Azure Devops and many others support code review functionality.
* IM - yes! for sanity check you can provide the code snippet on Slack/Hipchat or whatever IM you are using.
* Offline - Pair reviewing, ask your peer to review your code!

#### How?
* Look for working code over perfect code.
* New feature/Bug fix:
  * Check for functionalities,
  * Check for code structures does it follow the coding standards/guidelines?
  * Is there any side effects for the given change?
* New/Dark code:
  * Check for functionalities
* Test coverage?
  * Easier for reviewer to understand how underlying code works.
  * Boost confident to the code.


#### Code review best practices
* Focus on the code, not the developer!
  * This will remove any sort of bias that you may have while reviewing other's code.
* Do not assume anything!
  * Ask for clarity if you have any confusion.
* If you have opinions/better alternatives, provide it as a feedback and ask what the code owner think about your thoughts.
* Discuss explicitly the good/bad/side effects about the changes.
* Resolve conflicts with positive attitude.
* Appreciate the developer for what he/she is doing.

#### PR Best practices
* Keep your PR small
  * Small line of code means less bugs.
  * Easier to review.
* Use explicit/descriptive commit message
* Provide explicit description in the PR, any todo or out of scope functionalities.
* Provide test coverage, at-least areas that are important.
* Be prepared to take negative feedback
  * Address it as complement. Don't take anything too personal.
  * Try to resolve the conflict as possible.
  * Add the discussion/resolution in the commit message for posterity.
* and last but not least, use emojis!

Here is an example [PR](https://github.com/php/php-src/pull/2541) of mine in [PHP-src] (https://github.com/php/php-src) codebase that has almost everything I've said above!

![pr-image](https://cdn-std.dprcdn.net/files/acc_722737/awf2yP)

Code review is a great way to learn from others, be part of it and make it a habit.

Cover image credit: https://c1.staticflickr.com/3/2203/2245445147_ff54c5997d.jpg