DEV Community

Cover image for Code Review - let's do more!
Shaikhul Islam
Shaikhul Islam

Posted on

Code Review - let's do more!

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 all open PRs

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 of mine in PHP-src codebase that has almost everything I've said above!

pr-image

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

Top comments (0)