One of the core values of Avisi is quality. The software we create is of measurable quality. This quality comes from our passion to deliver great code in order to create the best solutions for our clients. One of the most important things we do to improve quality is code reviews throughout merge requests. A merge request (also known as pull request) is the request to review newly written code by several colleagues before the code becomes a definitive part of the software. These reviews make sure the code is checked whether it is correct, has been written clearly and meets the agreed standards. Only when everything is in order the reviewer agrees to 'merge' the code. Finally new code can be merged with the existing code.

Recently, I came across a few articles that caught my attention. One was about how to make the reviewer of your code happy. The other one was about why your team would not need merge requests. These articles stimulated me to write this blog about why I think reviewing code with merge requests contributes to the quality of the code the team delivers.

Merge requests are regular review moments

A merge request is a regular review moment to check if your teammates still have the same standards and values about testing, quality and code style. It also ensures that these are and remain uniform within the project.

It is important that the team is aware that a review is meant to improve together. Some people will find it less pleasant to receive criticism from their own team members, but if the common goal is clear, it is easy to explain.

In contrast to a periodic review for example, the review moment comes very quickly after the code has been written, making it a lot easier for the developer to make adjustments.

How to make it work:

• Before submitting a merge request always use tooling like automatic code formatting and linters to stick to the essentials.
• Use cohesive commits with a clear commit message. For example split refactoring of code and adding logic in separate commits.
• Keep your merge requests as small a possible: ask yourself as a software developer regularly at what point your changes will not break the existing software and does not contain too many changes at the same time.

Merge requests are educational

The educational part is not only for those who have their code reviewed. The reviewer learns from seeing how others solve problems. It does not need further explanation that this fact makes it very educational to do a review for less experienced software developers. During the review you get to know the project you are working on, you see what others are working on and you build an overview of what has already been implemented. You will also learn about which colleague knows a lot about the different parts of the software.

How to make it work:

• Comment constructively. Why can something be improved and, above all, how should it be done according to you. For example if you don't like the name of a class, you should not only comment why you don't like it, but also include the alternative name you have in mind.
• Always take into account the next reviewer, who also wants to learn something from it. So don't close a review comment with 'as discussed' but provide the outcome of the discussion in the comments of the merge request.
• Provide a compliment about a nice piece of code every now and then.
• If you feel like you don't understand each other, talk to the developer directly. Then provide a brief summary in the merge request so the next reviewer also learns from it.

Merge requests improve test quality

Tests are an important means of ensuring that the code is technically correct and does not cause unexpected changes in functionality. These tests must also be written and the team must agree that they properly record the desired functionality. A merge request is a perfect means to do so. You cannot just rely on continuous integration tests to ensure your code quality, because the tests only ensure the new code is technically and functionally correct, but it does not say anything about how it is implemented. When starting a new project there are little or no continuous integration tests, making merge requests extra important.

How to make it work:

• The continuous integration environment must also execute the tests in the branch you are developing on.
• Do not open a merge request until all tests have passed.
  
  

Merge requests require team effort

It is important that merge requests are handled in a structured way. It shouldn't take too long for a review to be approved. As a submitter, you will occasionally have to ask people to review. Of course it is much nicer if a reviewer comes automatically, you can set a good example for this yourself. You can make agreements about this with your team and discuss it regularly. Of course a reviewer is taken from his work for a while, but on the other hand, it is also good to occasionally shift your thoughts to gain new insights. If you automatically review someone else's work, the other person will also be more inclined to do a spontaneous review. The benefits of merge requests mentioned in this blog will likely help increase enthusiasm about reviewing.

How to make it work:

• Don't delay improving your code, do it immediately!
• If a merge request shows that you have tackled a problem completely differently than others expected, that is a moment to realise that you should have asked a colleague earlier wether your solution direction is the right one.
• Ask the reviewer in person to re-review when you have processed all provided improvement possibilities. This way the reviewer does not have to monitor this himself and there will be less time between the review rounds, making the reviewer having the request fresh in his mind.
•  

Articles

https://mtlynch.io/code-review-love/#1-review-your-own-code-first (How to make your code reviewer fall in love with you)

https://infrastructure-as-code.com/book/2021/01/02/pull-requests.html (Why your team doesn't need to use pull requests)