Yalantis: iOS, Android And Web App Development Company

Code Review Via GitLab Merge Requests

We believe a code review is an intrinsic part of any product development. It is used as an effective measure to increase the quality of the product delivery and make development process more efficient. In this article, we are going to focus on how to improve the workflow by doing code reviews via GitLab merge requests.

Before we get to the point of how to do a code review, let’s try to figure what might happen if you ignore this important stage.

Diverse character of developers on the team

The projects that our company gets engaged with usually require 1-3 developers. Needless to say, any development team includes people with different levels of expertise. That’s why there are respective job titles — junior, middle and senior iOS / Android developer. It doesn’t mean to say that the code written by a junior developer is necessarily of a worse quality than that of a senior IT specialist.

However, people are different. Especially creative mobile app developers, this sphere of technology is so lucky to employ within its ranks. Programmers tend to write software differently. Therefore their pieces of writing can sometimes create issues when other developers try to figure them out.

The reasons of this can deal with lack of knowledge, lack of sleep, lack of coffee, maybe an excess of creativity or an expression of high motivation, difference in the structure of brain or particularities of the character, you name it.

But why can’t we give it a try and solve those divergent thinking problems? Code review is a good starting point!

If you don’t do a code review

  • Every developer writes the «personal» code for the «personal» use. This might be an advantage if the developer is building his own product without having to cooperate with other developers. Or he just prefers not to load his brain with the problems he created for other human beings to deal with. The latter are not going to appreciate that, as well as the client who might have to spend more money and time required to unravel the mysteries hiding in the lines of that code. 
  • Neither member of the team has a full picture of all the code that the project contains. Eventually, the code will act like a blind dog in a meat market.
  • There are some pieces of code in the project that no one on the Earth, except the author, has ever seen and might never see, no matter how hard they try. Sounds very intriguing, right?
  • Bad quality code will likely remain in this condition for a long time, if not forever.
  • Nobody is responsible for the code of the others and sometimes for their own code either.
  • Members of the team find it hard to gain experience and improve skills in the code writing craft.
  • Bad code shoves in the testing phase.
  • Bad code accumulates itself. Sooner or later it will lead to many difficulties in trying to detect and fix it.
  • Junior developer runs the risk of being responsible for all the grief of the world.
  • When a new developer joins the project, his code might represent the biggest danger your software has ever experienced.
  • One and the same issue is solved differently by different developers. So there is no such a thing as code reuse.
  • It might be very complicated to do the refactoring after three months of producing the code that is out of control and lives its own life.

As you see, the perspective is not the most pleasant if you neglect doing a code review.

Read also: Mastering UIKit Performance

General principles of code review

  • Code review is an integral part of any development process — print it out and put on your wall to remember.
  • Code review is performed over small logically complete pieces of code, such as feature, task, bug fix, improvement, etc.
  • Only the code that has passed the review is sent to the testing department.
  • All the developers on the project do a code review regardless of their level.
  • All the developers on the project should pass a code review regardless of their level (junior developers should also review the code of the experienced middle and senior specialists).

Now we are getting close to what tools GitLab offers to make it possible to do a code review.

GitLab Merge Request as a tool to do a code review

A merge request is meant for merging the code from one branch to another.

The main merge request parameters (specified when creating):

  • source branch 
  • target branch
  • title
  • description
  • assignee

Code review via merge request in GitLab Read also: Lightweight iOS View Controllers through separate data sources

The course of actions when working with merge requests

  1. Write code and push it to a separate branch.
  2. Create a merge request for the main branch of development. An assignee and those mentioned in the description field and in the comments will be notified about the created merge request by email. In order to mention a developer, you should enter the symbol @ into the description field.
  3. Wait until it is accepted or declined with the comments about the necessary fixes.
  4. Take part in discussions about fixes. (GitLab allows to respond to comments)
  5. Fix.
  6. Push changes to your branch.
  7. Open a new merge if the last one was closed (if the merge request wasn’t closed, it will automatically update till the last commit at push).
  8. Report about the implemented fixes by commenting to the merge request or in some other way (Skype or directly).

Code review via merge request in GitLab

Error 500. What to do when GitLab merge request is not working

It may happen that GitLab returns Error 500 while a developer is trying to create a merge request. It means that GitLab server is not configured correctly. To solve this issue you would need some help from the system administrator (at least, this is how it works in our company) or someone who performs the role of a system administrator in your development team. There can be many different reasons why Error 500 comes up on the screen and there is no universal solution to this problem. However, in most cases you can solve this issue in one of the two following ways (or by trying both ways):

1) set up a value bigger than default for such parameters as max_size and timeout.

Max_size parameter sets up the maximum size of repository on GitLab;

Timeout is a response time. If during this interval the data weren’t returned, you get Error 500. Timeout is also the maximum time of completing a merge request.

How we solved the issue:

grep cryptspirit config/* -r

config/gitlab.yml:    max_size: 104857600 # 100.megabytes cryptspirit

config/gitlab.yml:    timeout: 20 # cryptspirit

Some additional reading here.

2) you can delete the repository of GitLab satellites and launch them again.

Clean up —>

rm -rf /home/git/gitlab-satellites/{repo}

Generate again —>

The last command should complete in GitLab directory with the configured gemset. Hope this helps!

Who should be assigned a merge request to

There can be different options depending on the number of people doing the project and their level of expertise. So if you are the only developer on the team, assign a merge request to yourself. After all, you can find bugs in your own code if you try to look for them. And remember, who seeks will always finds!

Otherwise, talk to another developer who is also on his own on the project and offer him to review each other’s code.

If you are two developers on the project, assign merge requests to each other. And if there are three or more developers, you are free to choose:

  • do it one by on
  • find any two developers crazy about merge requests and assign to them
  • assign all the requests to one developer and do a code review once a day with all the members of the team
  • any other option

You can do a code review at the beginning and at the end of the work day or anytime on request. The team can decide when it’s a good time to do a code review, the most important thing is a constant collaboration within the members of the team.

Read also: When testing guarantees bugs minimization

If you do a continuous code review

  • Problems in the code are detected and corrected right away
  • Every member of the team, not just the author of the code is responsible for its quality
  • Sharing knowledge happens much faster and more effectively
  • Junior developers learn faster
  • Only good code goes to testing
  • Bad code doesn’t accumulate
  • The code of a new developer on the project is no threat to the overall quality of code
  • Developers know well not only their own code, but the code of the whole project
  • The final product is of a good quality

You can watch the video describing issues, merge requests and integrations in GitLab.

A word on quality

Quality is one of our most important values. Learn how we keep the quality of our work

Testing and Quality Assurance at Yalantis
Written by Rustam Tagiev

Testing and Quality Assurance at Yalantis

According to the Agile methodology that we follow, every iteration in the process of app development in...

Quality Standards at Yalantis
Written by Kate Abrosimova

Quality Standards at Yalantis

We control the quality of our services continually and incessantly. Quality assurance isn’t a job assigne...

How Code Quality Is Measured: Android Code Review at Yalantis
Written by Kate Abrosimova and Aleksey Shliama

How Code Quality Is Measured: Android Code Review at Yalantis

Our code analysis includes six code quality checkpoints, a code review checklist, and a document for suggesting improvements.

MPAC – A Way to Control the Quality of Your Team's Work
Written by Ian Chernov

MPAC – A Way to Control the Quality of Your Team's Work

MPAC is a term coined by Yalantis, and used to help product owners maintain control over their projects.

Ready to work with us?

We have everything you might ever need to create a mobile solution that can attack the market and win.

Contact us