Home | Blog

Importance of Code reviews

Cover Image for Importance of Code reviews
Matija Kovacek
Matija Kovacek

Inspired by some of the last few projects, I have noticed that still a lot of people don't consider Code Review seriously.

From extreme cases where code review doesn't exist at all or where pull/merge requests are approved in a few seconds after being opened. Over the cases where the minority of team members care and perform code reviews, and the rest of the team members ignore them. The funny thing is when they are explicitly asked to check it, they consider them as "punishment".

Luckily, one of my first projects had a really good code review process where all team members were very active and performed strict but fair code reviews. On that project, I learned a lot, learned why this process is so important and gained really good habits on how to perform code reviews.

That's probably one of the reasons why I'm writing this today.

Recently I was preparing guidelines and best practices about code reviews so I decided to share them with you as well.

The things you will see here are not only my statements, they are inspired by other people, teams, and companies. But with those statements, I totally agree and I stand behind them.


So what is code review?

A code review is a process where someone other than the author(s) of a piece of code examines that code. Code review should be used to maintain the quality of our code and products.


Process Flow

Once you're satisfied and proud with your implementation and the tests, you create a pull request. With a pull request, you are requesting the merge of your local branch into the main source branch.

All dev team members (including Leads / Architects) should be default reviewers of the pull request. The reviewer's job is to check if all requested functionalities are properly implemented and tested. She/He can suggest code improvements like using some library instead of reinventing the wheel, better naming for components and variables, and any other improvement that she/he can suggest.

It is important that you shouldn't be disappointed if you receive a lot of comments on your pull requests. You will learn from the feedback and the solution will be better.

When the minimum required number of reviewers have approved the pull request you can merge it. Don’t forget to delete your source (feature/bugfix) branch and move the ticket that is ready for QA (after it's deployed).


Author of the pull request

Goal: Learn, improve your code quality, morale, and your working relationships.

REVIEW your own code first

Before sending code to your teammate, read it yourself and check the things that you would check as a code reviewer.

Be UP TO DATE with the develop branch state

The recommendation is always to be up to date with the latest state from develop branch. This means that during the day you should pull changes several times and merge it to your local feature branch.

Before creating a pull request make sure that your feature/bugfix branch is up to date.

Pull request should be DESCRIPTIVE

  • Branch Name
    • Source branch name should consist of branch type (feature, bugfix, hotfix), Jira ticket number, and Jira ticket title
      • e.g feature/ticket123-some-ticket-title
  • Commit messages
    • Commit early, commit often with a descriptive message what you did
  • Pull request description
    • Add an additional summary of what is being done if it’s not clear from the pull request title
    • Point out not so obvious/logical decisions and agreements
  • Comment your decisions
    • Comment yourself (inline in code) for not so obvious/logical decisions, so that reviewers can easily understand it

Prefer SMALLER pull requests

Try to make your pull request small as possible.

Smaller pull requests:

  • easier to review and understand
  • faster to review
  • less chance to miss something
  • less chance to create a bug

Respond graciously to feedback and LEARN from it

Don’t take personally a code review feedback. Treat your reviewer’s notes as an objective discussion about the code.

Be PATIENT when your reviewer is wrong

From time to time, reviewers are wrong. Just as you can accidentally write buggy code, your reviewer can misunderstand the correct code.

COMMUNICATE your responses explicitly

For every comment that requires action, respond explicitly to confirm that you’ve addressed it. Some code review tools allow you to mark comments as resolved. Otherwise, follow a simple convention, like, "Done", for each note. If you disagree with the note, politely explain why you declined to take action.

Don’t directly ask/chat with the reviewer about every comment. Commenting and discussing directly in the pull request is useful for other reviewers as well.

MINIMIZE lag between rounds of review

Once you create a pull request, driving the review to completion should be your highest priority. Delays on your end, will waste time for your reviewer, and increase complexity for your whole team.

RESOLVE Conflicts

In case of:

  • disagreements
  • totally wrongly implemented features
  • too many mistakes
  • doubts, unclear things, or comments

schedule 1 on 1 call with the reviewer to resolve all doubts and unclear things. Make sure that you comment back agreements directly in a pull request for other reviewers.


Code reviewers

Goal: Mentor, educate and improve your working relationships.

What do Code Reviewers look for

  • Design
    • Is the code well-designed and appropriate for your system?
  • Functionality
    • Does the code behave as the author likely intended?
    • Is the way the code behaves good for its users (end-users and developers)?
  • Complexity
    • Could the code be made simpler?
    • Would another developer be able to easily understand and use this code when they come across it in the future?
  • Tests
    • Does the code have correct and well-designed tests?
    • Do the tests cover requirements?
  • Naming
    • Did the developer choose clear names for variables, classes, methods?
  • Comments
    • Are the comments clear and useful?
  • Style
    • Does the code follow the company style guide?
  • Documentation
    • Did the developer update relevant documentation?

Don’t criticize, MENTOR and EDUCATE

Code review is a chance for developers to mentor and teach others.

When you comment on someone's pull request:

  • be kind
  • instead of criticizing, ask why decisions were made like that
  • suggest improvements
  • explain your reasoning
  • balance giving explicit directions with just pointing out problems and letting the developer decide

PRAISE good code

Remember to praise good code and decisions. Don't only point out problems and suggestions.

Don’t ignore pull requests

Don’t ignore pull requests and think someone instead of you will do the review.

Make a habit of doing code reviews like in the morning before daily, around lunchtime, before the end of the working day.

If you are in the middle of a focused task, such as writing code, don’t interrupt yourself to do a code review. Instead, wait for a breakpoint in your work before you respond to a request for review.

Less experienced team members don’t be shy

Your opinion is important and expected here as well. More experienced team members don't know everything and they also make mistakes.


Speed of Code Reviews

The speed of individual development is important, but not as important as the speed of the entire team.

The speed of the team as a whole will be decreased if the individuals don’t respond to the pull requests, do the review, and push other work (task) in the done direction.

If you are not in the middle of a focused task, you should do a code review shortly after it comes in.


Code Review Improvements Over Time

If you follow these guidelines and you are strict with your code reviews, you should find that the entire code review process tends to go faster and faster over time.

Developers will learn what is required for healthy code, and when you create a pull request that is great from the start, it will require less and less review time.

Reviewers will learn to respond quickly and not add unnecessary latency into the review process.

But don’t compromise on the code review standards or quality for an imagined improvement in velocity—it’s not actually going to make anything happen more quickly, in the long run.

References:

Read more

Cover Image for Optimizing slow Unit Tests

Optimizing slow Unit Tests

Understanding the motivation behind optimizing slow unit tests is crucial. We'll explore the challenges faced by Client XYZ, why we wanted to fix them, and the good things that happened afterward. Expect insights into how faster tests can boost productivity and project success.

Matija Kovacek
Matija Kovacek
Cover Image for Importance of Code reviews

Importance of Code reviews

Inspired by some of the last few projects, I have noticed that still a lot of people don't consider Code Review seriously. So what is code review? A code review is a process where someone other than the author(s) of a piece of code examines that code. Code review should be used to maintain the quality of our code and products.

Matija Kovacek
Matija Kovacek
Cover Image for AEM API Integration with Feign HTTP client

AEM API Integration with Feign HTTP client

How to call RESTful Web Service in AEM? Luckily there is Feign HTTP client which simplifies REST API Integrations. Check out how to integrate it in AEM project.

Matija Kovacek
Matija Kovacek
Cover Image for Speed up the Maven Build Time

Speed up the Maven Build Time

How to speed up Maven build time? 2x time faster Maven build time with Maven Daemon.

Matija Kovacek
Matija Kovacek
Cover Image for Test behaviour, not implementation

Test behaviour, not implementation

Test behaviour, not implementation if you want to build right product. For your own good it will save you time and money.

Matija Kovacek
Matija Kovacek
Cover Image for Really? Preselected checkbox not working, common AEM?

Really? Preselected checkbox not working, common AEM?

Why one simple preselected checkbox doesn't work in page properties?

Matija Kovacek
Matija Kovacek
Cover Image for Updating AEM content with Sling Pipes

Updating AEM content with Sling Pipes

You are still updating content manually? Try out Sling pipes. Sling pipes is simple tool for executing CRUD operations over resources in AEM repository. But is it powerfull enough to replace your groovy scripts?

Matija Kovacek
Matija Kovacek