How we improved code review process in Android engineering team
Here, at Azimo, we know the value of fast product delivery. That’s why, for many years, we’ve been delivering a new version of our mobile app once a week, so our customers have access to the latest features, bug fixes, and a variety of improvements. Recently we decided to be even faster and enable our engineering team to deliver changes to production within one day after code is done.
While a once-a-week release is already ambitious, in practice, it means that many of our new features or bug fixes need to wait days before reaching customers. Unless you have a good reason for that (e.g., feature parity across all platforms), it shouldn’t be like that. Our customers deserve immediate access to all the things we build.
Making Android app delivery a couple of times faster (from seven days to one) requires ground changes in the entire development process — things like:
- code review,
In this article, we will cover code review. For testing, check our latest series of blog posts about The evolution of apps Quality Assurance at Azimo.
Improving code review process
In our Android engineering team, before a code is merged to the master branch, it must be approved by each of the available team members. Thanks to that, not only our codebase is better quality. A code review is an excellent opportunity for knowledge sharing between engineers who work within different mission teams (different features, different domains).
But our engineering team’s configuration is also challenging for us in terms of checking merge requests. Each mission team has only one Android engineer, so every developer focuses on their job. They don’t actively wait for you to raise a chunk of code for review.
Until now, our process was far from being perfect. When the code was ready to be reviewed, an engineer dropped a message on Slack and waited patiently for others to check it. It was long hours of thinking — “Ok, I got Juliet’s approval, but what about Tom? Should I ask him again? Is he working today? Maybe he’s not, and I should merge it?”.
It wasn’t comfortable for Tom, especially when he needed to answer “No, not yet” multiple times.
To resolve these challenges, we decided to build Mr manager — our solution for filling the communication gap between authors of merge requests and reviewers. Here we talk about its features that make our life easier and the review process well organized.
Who hasn’t reviewed it yet?
The core feature of Mr manager is to look for open merge requests which weren’t checked by all of the team members. These are MRs which either aren’t approved or don’t have any comments from a particular engineer.
After identifying them, our solution sends a private message with a link to merge requests and randomly selected hints from a great article about the code review process by Philip Hauers. Like “Don’t forget to praise” or “Be aware of your personal taste, accept other solutions and make compromises.”
Keep the discussions short
Long discussions are another pain point of code reviews. Sometimes it takes days to resolve a thread with several opinions and a few people involved. We believe that it’s better to talk about those issues in person and try to find the proper solution together. That’s why our Mr manager searches for long discussions and creates private conversations with all involved engineers.
Ready to merge
When review, discussion, and testing are done, we should merge code to the master branch. You will be surprised by how many open merge requests are just waiting for being finished by the “merge” button click. That’s why Mr manager identifies these and lets the author know, so she or he can do the honors of closing them.
Here is the list of rules that identifies our ready-to-be-merged requests:
- merge request is not prefixed with “Draft:” nor “WIP:”,
- every working team member thumbed up the merge request,
- all discussions are resolved.
How does Mr manager work?
Mr manager checks our merge requests at the beginning, in the middle, and end of our working day. It skips all non-present developers and doesn’t touch merge requests prefixed with “Draft:”.
Is Mr manager intrusive for our engineers? No, because we treat it only as a reminder. Actually, it’s much easier to ignore these messages than the ones from our colleagues. We don’t feel as bad as we did when telling for the 5th time — “I’m sorry, I haven’t had a chance to do it yet ☹️.”
Mr manager is entirely written in Python and triggered by Jenkins cron pipeline. The script uses Python Gitlab SDK to fetch information about merge requests, project members, comments, discussions and emoji reactions added to comments or merge requests. Engineers’ availability is checked through the Slack Python SDK by analyzing their Slack statuses. Once the Mr manager has all the information, it finds out who should be informed about what, composes messages, and sends them to proper people on private chat.
We also created a separate Slack bot with its own channel. Thanks to that, we have the current status of merge requests available, so we don’t have to visit the Gitlab page to check whether something requires our attention.
Does it work?
Simply, yes! After a few days of using the Mr manager, the feedback in our team is really good. It removed a few manual steps, made our merge requests list shorter, and improved the speed of delivery.
Right now, we can easily connect any repository in Azimo to the Mr manager. In the future, we plan to use Gitlab webhooks to trigger some Mr manager checks and let it deliver the proper message immediately, not three times a day. Stay tuned for these and others updates.
Towards financial services available to all
We’re working throughout the company to create faster, cheaper, and more available financial services all over the world, and here are some of the techniques that we’re utilizing. There’s still a long way ahead of us, and if you’d like to be part of that journey, check out our careers page.