Codeisok, or History of code review at Badoo / Badoo's Blog

Hello reader! My name is Ilya Ageev, I have been working at Badoo for almost seven years. My current position is called Engineering Director Quality Assurance, and in fact I do a lot of things in the company: quality control, release engineering, development processes and methodology, and even a bit of information security. One area that falls within my area of ​​responsibility is code review. Today I will tell you about the history and the result of our process (which became the open source codeisok ).

The article contains a historical description; there is no technical comparison of various tools for code review in it. Nevertheless, I give examples of such tools, and if you are interested in the question, then you may well try to independently install and compare tools.

Once upon a time in Badoo there was no code review. In practice. The first attempt to exchange information about the changed code was to simply send the changes to the mail. By the way, Git was not used then either.

In the summer of 2011, when I first joined the company, SVN was used to store versions of the code. Experienced said that before that everyone worked in CVS and that there was one attempt to switch to Mercurial, but it failed. Why - no one remembers. I suspect that the fact is that when they decided to try Mercurial, there was no particular need to switch to another VCS . Everything suited everyone: it works, and okay. And for the sake of fashion no one needs a fashionable tool.


In any case, I did not find those times, and my acquaintance with the project is closely related to SVN. Development was carried out in one branch - trunk. Sometimes third-party experimental things were done in separate SVN branches with subsequent painful attempts to reduce what happened to a common branch. It is not surprising: SVN is not the most ideal thing for working in branches.

The flow of work looked something like this: we take the trunk head, make changes to the code, commit (SVN accepts commits immediately to the server, into the common code) and we hope that everything will be fine. We remember the revision number received from SVN, and write it in the ticket, so that it is clear in the calculation what tasks got into it. Obviously, if the revision number in the ticket is less than the number laid out on the production revision, then the code for this task is in the history of commits.

After the commit, the diff with the changes was sent to the mail, to the general mailing list. Thanks to this, those who wanted to get acquainted with the code changes could do this. Naturally, there was no streamlined code review process.

Git

As the head of the QA department, who just came to the project and was preoccupied with the correct construction of the process, it immediately became clear to me that it was impossible to live like that further. Code that is developed in one branch, which is difficult to analyze for problems and errors, which is hard to lay out and cannot be rolled back, is completely unsuccessful for good testing.


Accordingly, the first thing to do is to separate the flies from the cutlets. The result was a switch to Git and the ideology of feature brunches with all the attendant benefits of this approach. I spoke in detail about this transition at RIT ++ in 2012 in a pair report with Yuri Nasretdinov .

In this case, it was necessary to somehow track the changes made in the code for the task, and even more so in the release branch. To understand what has been done, how to test it, lay it out, find the roots of errors, etc., we did two things:

1. They left the "old" mechanism for notifying about changes in the form of a letter to the mail. By the way, this approach is still used, many of our engineers find it convenient: at any time in the mail you can find the changes made by a specific person in specific sections of the code and in a specific task. Today at Badoo, this mechanism works in parallel with the regular code review process.
   
2. I installed the code viewer in the central Git repository, where it was possible to see branches in the repository, commits, files, etc. When the task code got into the general repository, a link to changes to this web interface was registered with a Git hook so that everyone interested could follow the link and see the changes.
   

At that moment, GitPHP, an almost complete clone of cgit , seemed to me the most suitable tool for this purpose . I liked the tool because its minimalistic interface made it possible to do everything that we needed, and the open source code in PHP allowed us to change / add something as needed.

The tool domain http://gitphp.org/ , unfortunately, is dead for today. The repository on GitHub has also been deleted - apparently, the author no longer wants to develop and maintain the tool. But we remember https://github.com/xiphux/gitphp and are very grateful to the author - Chris Han.

We love and respect open-source, and in GitPHP, like in many other tools we work with, we made several improvements. Here inIn this article of November 2013, for example, we talked about how we optimized the speed of a tool and how Chris Han accepted them into the main code.

Code review

We switched to Git, set up the process. What's next? Another point that strategically affects many things in development is code review. The process, which has a lot of pluses (there are minuses too, somehow I will describe them in a separate article), was not built in our company.


We began to test different tools for organizing code review, did trial installations and studied the issue. First of all, we wanted to organize everything for the main Badoo repository with PHP code, and only then, if everything works out well, distribute it to the whole company.

The most important requirements that we made for the process and the tool at that time were:

1. The ability to host the tool within the company. We have several developments that are intellectual know-how, and from the point of view of common sense, I do not really want the source code to leak into the Network.
   
2. So that the code can be viewed / revised / checked immediately. And immediately give feedback. Do not wait until the developer formally indicates that everything is ready, as is done with pull requests in GitHub, for example. Moreover, we do a lot of basic checks automatically at the time of pushing the code into the general repository.
   
3. The implementation process should be transparent, smooth and should not interfere with current tasks that are already on the assembly line - so that no one notices any special changes, but, on the contrary, people get new opportunities if necessary.
   
4. Integration with Jira, which at that time was already widely used in the company.
   

At this stage, it is necessary to clarify two new terms that we use and which differ from the ideology of pull requests. This is a brunchdiff and a brunchlog. In fact, this is just the difference in code between the base branch (we have it master in most cases) and the task branch. Branchdiff is a complete diff between two branch heads. Branchlog - a complete log of commits showing the difference between the heads of two branches. As the trunk of the branch commits grows, the tasks of the branchdiff and branchlog can grow.

Gerrit

Примерно в то же самое время, когда я разбирался с основным репозиторием кода и PHP’шниками, наши сишники озаботились проведением code review в своих репозиториях (их у них много) и самостоятельно решили опробовать Gerrit. Антон Довгаль вот в этой статье подробно рассказывал о том, как это было, какие инструменты ещё рассматривались и какие проблемы хотелось и удалось решить.

Забегая вперёд, скажу, что сегодня Gerrit в Badoo не используется. А на тот момент для других отделов не подходил, так как умел делать ревью только на покоммитные изменения. Ревью кода всей ветки (бранчдиффа) инструмент делать не умел. Честно говоря, я не уверен, что умеет и сейчас. Поэтому от Gerrit как инструмента для ревью кода во всей компании мы отказались.

GitLab / GitHub

Back then, the head of the back office, Boris Chernetsov, helped us learn the tools for code review . Borya put different tools, showed how they work, proposed a review process supported by these tools, etc.

But none of the tools considered at that time were suitable for us (the standard phrase is justification for inventing your own bicycle). As far as I remember, the main problems were:

1. Many tools did not offer self-hosted solutions. And for us it was one of the basic requirements.
   
2. The absence of pre-receive hooks, which delayed validation and lengthened the development cycle. All automatic checks could be done only after the fact, after the creation of the pull request. This is how GitHub works, for example, which is also a paid service.
   
3. GitLab was then very raw and contained a bunch of bugs, although at first we really liked it. A lot of the declared functionality just did not work. Today, as far as I know, the guys have gone far ahead and developed the project well.
   

We examined a few more tools like Gitorious and others, but they were much inferior in functionality even to GitLab and did not provide many of the functions we needed.

GitPHP for code review

Disappointed in the tools that we managed to find, we decided to go a completely different way. Since there are no tools that are 100% suitable for us, and editing GitPHP code is not such a difficult task, why not try to make the simplest mechanism for commenting code in GitPHP? Moreover, all the other things we need to store the code on the server, control access Gitosis , link to tickets, view changes to branches, etc. - already exist.

They decided on that.

It took us very little time to create the first version. Nikolay Krapivny helped us make authorization through Atlassian Crowdand the simplest form of commenting out lines of code in a couple of weekends. And we realized how good and convenient it is: you click on the link in the ticket (the one that was already there before, which everyone is used to) - and you can write comments on the code.

Comments are sent to the developer by e-mail and can be placed on public mailings. Also added to the ticket in Jira. Very comfortably. To bother with creating separate "tickets for reviews" (as Crucible does , for example), assigning reviewers inside GitPHP (a reviewer is the one to whom the ticket was transferred after development in Jira), a separate review status, etc. is not necessary. There are comments, the ticket was reopened - that means the code needs to be fixed. If all is well, just transfer the ticket further.

This simple scheme so organically fit into the project and the developers liked it so much that it was quickly and easily picked up and started to be used.

Crucible and FishEye

But despite the fact that the backend developers quickly picked up the idea and began to widely use the tool, and some even helped to develop it, not everything was perfect.

Mobile developers at first strongly resisted and refused to use the tool. The excuses were different: starting with the fact that the interface is not pleasant, ending with the fact that it does not have any pieces he urgently needs.

We reviewed the code review tools on the market several times, trying to find something suitable in the hope of improving the situation. Tools really develop, they add a lot of useful and not very features, new ones appear. But new problems arise with all this.

So, the following review tools participated in our research: Upsource, Crucible, GitLab, Phabricator, Review Board, Kallithea and several others. Unfortunately, the ideal tool was not found among them.

Despite this, we still put Crucible in parallel with GitPHP and compared the work of these two tools for a whole year. Crucible was used by many mobile developers in their previous projects and gave positive feedback. He had one of the best integration with Jira, which is not surprising - he is also made by Atlassian.

The main Crucible problem, as it turned out, is performance. We even had to greatly reduce its functionality by disabling several features.

We tried different hardware, different optimizations recommended by the developers, but did not achieve anything. The fastest thing we managed to squeeze out of this tool was ten minutes before the code appeared in the tool after the push. And in the case of some tasks, the dependence of which was not clear, and a couple of hours had to wait. After increasing the number of repositories and commits, the speed fell nonlinearly. For our dynamics of development and delivery of features, this was not suitable.

The index that Crucible builds and stores internally, and which serves as the basis for displaying diffs, logs, and blames , is incredibly slow to build. We even tried different database engines. Sometimes, for some unknown reason, Crucible also began to “suspend” Jira. It was treated by restarting both systems.  

Well, in the end, it turned out that GitPHP also works many times faster, and we managed to add many Wishlist at the request of mobile developers, and Crucible has a lot of its problems. Accordingly, after playing with Crucible for a year, we made a strong-willed decision to implement GitPHP among mobile developers as well.

Today, the tool is used in our company as a standard in all departments. And the users who played with Gerrit use this tool, and mobile developers, and PHP developers, and JS developers with layout designers - in general, all-all-all.

Codeisok

Of course, since we started working with GitPHP, a lot of water has flowed, and we almost unrecognizably changed the original tool. He received new functionality, a new design, a bunch of additional features, optimizations and improvements.


We have added several authorization methods: not only through Jira and Crowd, but also through Redmine, for example. We changed the syntax highlighting and completely rewritten the side-by-side mode. We got rid of Gitosis and implemented the management of repositories, users and their access via the web interface - easy and convenient ( Sasha and Zhenya , thank you). We added filters for files ( Sasha , thanks), extensions, changes in diffs and reviews. We made the installation of the instrument easy by wrapping everything in a docker . We completely changed the design. Well, we don’t forget about the ability to do code review itself - it also evolved and became much more convenient.

In general, the tool became completely different, so we gave it a new name: Codeisok. It reads "code from ok" and means that everything is fine with the code.

The expression is an internal corporate meme and is used in dialogs:

- Do a review of my task, please.
- Looked, codeisok'nul!

And today, we are pleased to announce that Codeisok is available to the open-source community. The tool already works in several projects outside of Badoo and has positive reviews (for example, wetrend.com and turbocontract.ru ). We hope that you will also like it and be useful. We will be happy to accept improvements and pull requests - we are always glad to those who help to develop open-source tools.

Also, taking this opportunity, I want to express my deep gratitude to everyone who helped to develop and improve the project. Guys thanks! You are incredibly cool!

Nikil Verma
Stas Ignatenkov
Alexander Izmailov
Anton Dovgal
Yuri Nasretdinov
Nikolay Krapivny
Konstantin Dzyuin
Milosh Popovich
Alexander Treger
Evgeny Makhrov
Denis Korenevsky
Pavel Matolygin
Alexander Grishin
Roman Lazarev
Alexander Ovsyankin

In code we trust!
Ilya Ageev, Badoo