Codeisok, or History code review in Badoo

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 of the areas that is in my area of ​​responsibility is review of the code. On the history and the result of our process (which has become an open source codeisok ) I will tell you today.

The article contains a historical description, there is no technical comparison of different tools for reviewing the code. Nevertheless, I give examples of such tools, and if you are interested in a question, you can easily try installing and comparing tools yourself.

Once upon a time in Badoo there was no code review. Almost. The first attempt to exchange information about the changed code was a simple mailing of changes to the mail. By the way, and Git was not used then.
')
In the summer of 2011, when I first joined the company, SVN was used to store code versions. Experienced people said that before this 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 Mercurial was decided to try, there was no particular need to switch to another VCS . Everything was fine with everyone: it works, and okay. A fashionable tool for the sake of fashion is not needed by anyone.


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

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

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

Git

As the head of the QA-department, who only came to the project and was concerned with the proper construction of the process, it immediately became clear to me that it was impossible to continue this way. Code that is developed in one branch, which is difficult to analyze for problems and errors, which is difficult to deploy and impossible to roll back, does not contribute to good testing at all.


Accordingly, the first thing to do is to separate the flies from the chops. The result was the transition to Git and the ideology of feature - branches with all the attendant advantages of this approach. I talked in detail about this transition in RIT ++ in 2012 in a paired report with Yuri Nasretdinov .

At the same time, 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, lay out, find the roots of errors, etc., we have done two things:

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

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

The domain of the tool http://gitphp.org/ , unfortunately, is dead today. The repository on GitHub is also 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, as in many other tools with which we work, made several improvements. Here in this article from November 2013, for example, we told how to optimize the speed of the tool and how Chris Han accepted them into the main code.

Code review

On Git switched, the process is configured. What's next? Another point that strategically influences many things in development is the review of the code. The process that has a lot of advantages (there are also disadvantages, 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, made trial installations and studied the issue. First of all, we wanted to organize everything for the main repository of Badoo with PHP code, and only then, if everything goes well, distribute it to the whole company.

The most important requirements that we set 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 we don’t really want the source code to be leaked to the Web.
   
2. So that the code can be viewed / reviewed / 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 moment of pushing the code into the common repository.
   
3. The implementation process should be transparent, smooth and should not interfere with current tasks that are already on the pipeline - so that no one will notice any special changes, but, on the contrary, people will 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 branch diff and branchlog. In fact, this is just a difference in the code between the base branch (we have master in most cases) and the task branch. Branchdiff - full diff between two heads of branches. Branchlog is a complete log of commits showing the difference between the heads of two branches. As the trunk grows, branches of the branchdiff and branchlog task can grow.

Gerrit

At about the same time when I was dealing with the main repository of the code and PHP's, our syshniki took care to conduct a code review in their repositories (they have a lot of them) and decided to try Gerrit on their own. Anton Dovgal here in this article spoke in detail about how it was, what tools were still considered and what problems we wanted and managed to solve.

Looking ahead, I will say that today Gerrit is not used in Badoo. But at that time it did not suit other departments, as he was able to make a review only for changes. I did not know how to make a tool to review the code of the entire branch (branchdiff). Honestly, I'm not sure that I can now. Therefore, we refused Gerrit as a tool for reviewing the code throughout the company.

GitLab / GitHub

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

But none of the instruments considered at that time came to us (the standard phrase is an excuse for inventing our own bicycle). As I recall, 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 that alienated checks and lengthened the development cycle. All automatic checks could be done only after the fact, after creating a 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 simply did not work. Today, as far as I know, the guys have gone far ahead and well developed the project.
   

We looked at 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 with the tools that we managed to find, we decided to go a completely different way. Since there are no tools that suit us 100%, and editing the GitPHP code is not such a difficult task, why not try to make the simplest mechanism for commenting code in GitPHP? Especially since all the other things we need are storing the code on the server, access control for Gitosis , linking to tickets, viewing branch changes, etc., is already there.

On that and decided.

It took us quite a bit of time to create the first version. Nikolay Krapivny helped us make authorization through Atlassian Crowd and the simplest form of commenting on lines of code for a couple of days off. And we realized how good and convenient it is: poking on a link in a ticket (the one that was already there, to which everyone is accustomed) - and you can write comments on the code.

Comments are sent to the developer by email and can be posted to the public list. Also added to the ticket in Jira. Very comfortably. There is no need to bother with the creation of separate “reviews for reviews” (as does the Crucible , for example), the appointment of reviewers inside GitPHP (the reviewer is the one to whom the ticket was transferred to after the development in Jira). There are comments, re-opened the ticket - it means that the code must be corrected. If everything is good, just translate the ticket further.

This simple scheme so organically fit into the project and the developers liked it so much that they picked it up quickly and easily and began to use it.

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. Excuses were different: starting with the fact that the interface does not like, ending with the fact that there are no badly needed pieces in it.

We did several reviews of code review tools on the market, trying to find something suitable in the hope of improving the situation. The tools are really developing, they add a lot of useful and not very features, new ones appear. But problems with all this are new.

For example, the following review tools were used in our research: Upsource, Crucible, GitLab, Phabricator, Review Board, Kallithea, and several others. Unfortunately, there was no perfect tool among them.

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

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

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

The index that Crucible builds and stores inside and which serves as a base for showing diffs, logs and blames is built incredibly slowly. We even tried different database engines. Sometimes for unknown reasons, the Crucible began to "hang" and Jira. It was treated with a restart of both systems.

Well, in the end, it turned out that GitPHP also works many times faster, and we managed to add many requests for mobile developers, and Crucible has a lot of problems. Accordingly, after playing a year with Crucible, we made a willful decision to implement GitPHP and mobile developers, too.

Today the tool is used in our company as a standard in all departments. And sishniki, which were played with Gerrit, use this tool, and mobile developers, and PHP'shniki, and JS'niki with typesetters - in general, everything, everything.

Codeisok

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


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

In general, the tool has become 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 intra-corporate meme and is used in dialogs:

- Review my task, please.
- Looked, codeisok'nul!

And today we are pleased to announce that Codeisok is available for the open-source community. The tool is already working in several projects outside Badoo and has positive reviews (for example, wetrend.com and turbocontract.ru ). We hope that you will like it too and will be useful. Improvements and pull-requests we will be happy to accept - always welcome those who help 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. Thanks guys! You are incredibly cool!

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

In code we trust!
Ilya Ageev, Badoo