In this article, we focus on how to improve the workflow with code reviews via GitLab merge requests. We also talk about code review best practices we follow at Yalantis. Before we get to how to do a code review, let’s figure out what might happen if you ignore this important step.
Developers on the team
Projects our company works on usually require one to three developers per platform. And nearly any development team includes people with different levels of expertise (developers are generally classified as junior, middle, and senior).
However, regardless of their level, all programmers write software differently. Especially mobile app developers, who tend to be creative. Therefore, it can sometimes be difficult for developers to figure out code written by their peers. This issue can also stem from a lack of knowledge, sleep, coffee, or an excess of creativity.
So how can we make sure that developers can read each other's code? Reviewing source code is a good starting point!
Why review code?
Many developers write code as if they were the only ones who ever had to understand it. This isn’t a problem if a developer is building a product on their own.
But such a world of peaceful loneliness doesn’t exist. The reality is that developers have to think about those who will take over their work. They also have to think about clients, who will have to spend money and time unraveling the mysteries hidden in lines of unclear code. Here are some reasons for doing a code review:
- No member of a team has a full picture of all the code a project contains, as it’s written by different people.
- Some pieces of code in any project will only ever be seen by the author, no matter how hard others try.
- Bad quality code is likely to remain in this condition for a long time, if not forever.
- Nobody is responsible for the code of others – and sometimes not even for their own.
- Team members find it hard to gain experience and improve coding skills.
- Bad code becomes a pain in the neck in the testing phase.
- Bad code accrues. Sooner or later, this will lead to many difficulties in trying to detect issues and fix them.
- When a new developer joins a project, their code might represent the biggest danger your software has ever experienced.
- It can be very complicated to do refactoring after three months of producing code that’s out of control and lives its own life.
As you can see, the prospects are not the most pleasant if you neglect to do a code review. But a code review alone can’t ensure code quality. First of all, code should pass through the testing phase.
Read also: Why Is Swift Faster Than Objective-C?
Testing to improve code quality
Code review can’t replace thorough test coverage and completely improve the quality of code. Before doing a code review, new code should have sufficient test coverage. At each push, GitLab pipelines carry out tests and code quality checks so a developer can fix issues.
While code reviews can, at times, be of help in finding bugs, they are not the primary tool for that purpose. A robust collection of unit, functional, and integration tests as well as code quality tools will effectively serve this purpose.
But there’s always the possibility of a bug not being caught by any test, and that’s where code reviews are critical. Insufficient testing can also produce uncertainty over the code’s business logic (whether it meets the initial requirements). And this is where a good code reviewer will step in.
As code is being written, the priority is to make sure that all tests are automated and run explicitly when new code gets pushed. A code reviewer sees when GitLab has completed testing a new piece of code and when that code is compiling.
The results of automated tests are clearly visible to developers. Passing the testing pipeline is a mandatory step before merging.
General principles of code review
- Code review is an integral part of any development process.
- Code review is performed over small, logically complete pieces of code such as a feature, task, bug fix, or improvement.
- Only code that has passed review is sent for testing.
- All developers on the project participate in code review regardless of their level (junior developers should also review the code of middle and senior specialists).
- Code of all developers should be reviewed, regardless of the developer’s level.
Now we’re getting to the tools GitLab offers for code review.
GitLab merge request as a tool for code review
A merge request is meant for merging code from one branch to another. The main merge request parameters (specified when creating a merge request) are:
- source branch
- target branch
- title
- description
- assignee
Read also: How Code Quality Is Measured: Android Code Review at Yalantis
Course of actions when working with merge requests
- Write code and push it to a separate branch.
- Create a merge request for the main branch of development. The assignee and those mentioned in the description field and in the comments will be notified by email about the created merge request. In order to mention a developer, enter the @ symbol in the description field.
- Wait until your request is accepted or declined with comments about necessary fixes.
- Take part in discussions about fixes. (GitLab allows you to respond to comments.)
- Make fixes.
- Push changes to your branch.
- Open a new merge request if the last one was closed. If the merge request wasn’t closed, it will automatically update till the last commit at push.
- Report implemented fixes by commenting on the merge request or in some other way (by messenger or directly).
Error 500. What to do when a GitLab merge request doesn’t work
GitLab may return Error 500 when a developer tries to create a merge request. This means the GitLab server is not configured correctly. To solve this issue, you’ll need some help from your system administrator (at least, this is how it works at Yalantis) or someone who performs the role of system administrator in your development team.
There can be many different reasons why Error 500 comes up, and there’s no universal solution to this problem. However, in most cases, you can solve this issue in one of two ways (or by trying both):
1) Set a larger value than the default for parameters such as max_size and timeout. The max_size parameter sets the maximum size of a GitLab repository. Timeout is the response time. If during this interval the data isn’t returned, you get Error 500. Timeout is also the maximum time for completing a merge request.
How we solved this issue:
grep cryptspirit config/* -r
config/gitlab.yml: max_size: 104857600 # 100.megabytes cryptspirit
config/gitlab.yml: timeout: 20 # cryptspirit
2) 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 be executed in the GitLab directory with the configured gemset. Hope this helps!
Who a merge request should be assigned to
The assignment of merge requests depends on various factors. There can be different options depending on the number of people on the project and their level of expertise. If you’re 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. He who seeks will always find!
Otherwise, talk to another developer who’s also on their own and offer to review each other’s code. Documentation review is also often necessary to be sure that other developers can work with your code.
If there are two developers on the project, assign merge requests to each other. If there are three or more developers, you’re free to choose:
- Assign merge requests to each other in turn
- Find any two developers who are crazy about merge requests and assign all requests to them
- Assign all requests to one developer and do a code review once a day with all members of the team
- Any other option
You can do a code review at the beginning and at the end of the workday or at any time upon request. The team can decide when it’s a good time to do a code review. The most important thing is to ensure ongoing collaboration within the team. How you do a code review depends directly on the practices your particular company follows.
Read also: Unit Testing for Web Software: Why It’s Necessary and Which Frameworks to Use
If you do a continuous code review make sure that:
- Problems in code are detected and corrected right away
- Every member of the team, not just the author of the code, is responsible for its quality
- Knowledge sharing happens fast and effectively
- Junior developers learn fast
- Only good code goes to testing
- Bad code doesn’t accumulate
- A new developer’s code doesn’t threaten 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 high quality
For more information, watch this video describing issues, merge requests, and integrations in GitLab.
Some companies use a code review app and review tools, but note that code review apps have their limitations.
Best practices for code review at Yalantis
To minimize the time spent on reviewing each merge request, you need to have a strategy for code review.
In our humble opinion, a good developer is not just someone who follows a programming workflow and writes high-quality code. A good developer knows how to deliver code for review and make the whole code review process effortless for the reviewer.
To improve the code review workflow, we follow a unified strategy for working with Git – the widely known Gitflow Workflow.
The Gitflow Workflow is a strict branching model designed around a particular project. It helps unify things like names of branches, the structure of commit messages, communication between reviewers and reviewees, and many other project details.
In addition to following such a workflow, keep in mind that a good merge request should solve a specific task. We suggest not including more than one feature in one merge request. It would be much better if you created several merge requests instead.
As a reviewer, don’t hesitate to pull the source branch and test incoming code by yourself, especially if the merge request contains plenty of changes. Build the project and check that everything works as expected.
Also, an important detail in our code review checklist is deleting branches when they’re no longer needed. The responsibility for deleting branches after they’re fully merged lies with the reviewer.
But as a reviewee, you can simplify your colleagues’ lives by selecting the checkmark to delete the source branch when the merge request is accepted. By doing this, the source branch will be automatically deleted right after the merge request is accepted.
As you can see, a code review is a handy tool to make development as efficient as possible. A code review is an essential part of app development, allowing for stable software, accurate quality measurement, and analysis.
Ten articles before and after
How to Speed Up JSON Encoding and Decoding in Golang
How to Deploy Amin Panel Using QOR Golang SDK: Full Guide With Code
Best Tools and Main Reasons to Monitor Go Application Performance
Which Javascript Frameworks to Choose in 2021
Using RxSwift for Reactive Programming in Swift
How to Create a Restful API: Your Guide to Making a Developer-Friendly API
Practical Tips on Adding Push Notifications to iOS or Android Apps
How To Choose a Technology Stack For Your Web App in 2021