How to reduce code review from two weeks to several hours. Experience team Yandex. Market

Man is the main value of any enterprise. The success of the whole business depends on how people communicate with each other, how they achieve their goals together, on teamwork. Continuous refinement of skills, processes and tools allows you to work more efficiently.

We in the Market are working to deliver new features to our users as quickly as possible. To do this, we constantly study our processes and optimize all stages of work on the task. Today we will tell Habr's readers about the optimization of one of them, namely the code review process.

First you need to imagine what flow in the development we have adopted:

The same thing, only by points and words:

1. The developer takes a task.
2. Makes it.
3. Fills in Github and opens PR.
4. Pass code review.
5. Collects a demo stand and gives it to the tester.
6. The tester checks the demo stand.
7. Proven tasku collected in the release.
8. Testing in release.
9. Prod.
10. Final testing in combat.

By areas of responsibility, the task is divided into the following steps:

1-5 - responsibility on the developer;
6-7 - responsibility on the tester;
8-10 - responsibility on the release master.

So, we began to analyze what takes us the most time. Fortunately there are internal analysis tools. Everyone relied on the fact that it would take the longest to occupy the development process itself (“In Work” status) ... and it turned out that way. But one moment surprised us the most. The average review duration is two weeks!

Step 1. Parsing the PR

Starting to study pull-rekvesy, immediately faced with one very interesting fact. It turned out that we have a huge cemetery of unclosed pull requests. Most of the authors of these PRs no longer worked for the company, but their legacy was still with us. Who never saw dinosaurs, here they are:

These pull requests added noise and interfered with correct code-review time analysis. With peace of mind, we closed them down. It only remained to count the time. And it was still around 2-3 days. Not good.

Action 2. Disassembling with a dishew

Revushnitsa - a system developed in Yandex, which calls in PR two random people with expertise in a variable code and taking into account vacations and absences. An analysis of weekly PRs revealed a group of individuals who are constantly detaining the code review. After interviewing our colleagues, we found another problem in our process. Colleagues complained that they received too many pull-requests per day from the reseller, and they simply do not have enough time for their main work.

This did not agree with our indicators: one or two PR per person per day. The study resulted in Github, where we have a separate team of reviewers. This command has not been updated for several years. Since then, the number of employees has doubled, but none of the newcomers have been part of the review team. In other words, half the team did not participate in the code review at all! Corrected this annoying misunderstanding.

Step 3. Expand tools

At this stage, we tried to simplify the life of the reviewers, which was already as simple as we thought. In the arsenal of front-fighters were the following tools:

• code style checkers;
• test run;
• various non-security checks in the PR itself;
• revisionist;
• alerts to mail and task tracker.

It would seem that everything is there. Take it and review!

The first thing that turned out to be wrong: a different code-review process in different repositories. They unified and affixed labels at the same time for easy PR search through the Github interface.

The second thing they noticed: mail is not the best tool to quickly inform you about the status of code review. Many, in order not to be distracted from work, parse their mail several times a day. Messengers are quite different. The response to messages in messengers is much higher. And it was decided to connect the bot to our messenger. The bot sends alerts about the status of the code review for both the author of the pull request and encourages people to mess around. Very comfortably.

Step 4: First SLA

In parallel with actions 2 and 3, we began to work closely with the staff and explain that the review code is inseparable from the task itself. We explained that bringing to “Verified” is the responsibility of the developer. And the responsibility is not only to colleagues, but also to users! Fast delivery features to sell - that's what I wanted to achieve. And the team treated with understanding the process of improvement.

At this stage, the first idea of ​​the ideal code review was born. Of course, everyone wants it to take place in 5 minutes, but this is not always possible. Taking into account that we have a multiregional team (with a time difference of four hours), we agreed that our SLA will have one day, i.e. 24 hours. This SLA was announced to all employees, and, rubbing their hands, they began to wait for results.

But the situation has not changed. In the best weeks, half of the pull-requests fit into 1 day. The others got out for him.

We had a small script that estimated the time for a code review on PR. On our daily basis, we began to grind all the PRs in search of the "lagging behind." Almost every day there was a pack of such.

It took a lot of time to analyze them. Most often, the script itself is dishonestly cheating time review. He did not take into account that some PRs created after hours (yes, we have some courageous ones who know how to work for an hour or two at night, come to work with us!). All this told us that our monitoring tools for pull-requests are not the most effective, because they are dishonest. We'll have to find new tools.

Step 5: SLA Tracker

Help came from no waiting. Our colleagues from Tracker announced an amazing thing: now you can install SLA in the Tracker itself. And you can customize it completely diverse. A certain timer is activated for some event in the task. For some event can pause. And for some event to stop. Yes, this is what we need!

We immediately went over to study the documentation in detail (by the way, here it is ) and configure our queues (there are several of them, since there are several repositories). They set up the timer to turn on when switching to the status of "Need code review" and complete when switching to any other status, except for "There are comments" - with it, the timer paused, i.e. did not reset your time.

It also turned out to be cool that the timer took into account the working time and calendar! We set that 9h is assigned to the code review, i.e. one working day. In this case, the alert is triggered 2h after the start, or if the nine-hour period has been broken. The result was a kind of honest monitoring. At first we turned on the timer for the sake of experiment, collected a pack of bugs and replayed it in the Tracker.

It is worth noting that the timer was included only for tasks that were created after the introduction of the timer itself. Therefore, the instant effect could not be seen. But already at this stage positive dynamics began. We received the effect a month later, when the flow of new tickets with a timer began to crowd out the old ones. It was noticeable that the approximate time with the code-review was concentrated in the area of ​​the reminder messages, i.e. marks 2h and 9h.

At the time of this writing, we have 415 tasks in which the timer is turned on. Of these, 29 went beyond the nine o'clock border. Although, if you look closely, there are such tasks, the code-review of which was completed within the next hour. If we drop them, there are 17 tasks left. And this is about 4.1%!

Total. Samurai constantly sharpens his sword

Looking back and remembering all our actions, we can draw one conclusion - all means are good. All our steps led to the desired result: more than 92% of pull requests began to satisfy our SLA ! Fewer tackles are stopping, the code review goes faster and faster. Little by little, 75% of the code review began to fit into 5 hours ! And the dynamics of improvement are still positive. In addition to numerical indicators, we began to receive more positive feedback from subcontractors. Even more pleased with the fact that the team reacted to the whole process. Even a process such as code-review acceleration has further consolidated the team. Each participant began to understand the responsibility that he bears to others, as a fast code review allows us to benefit even more quickly.

Of course, 9h is not the ultimate dream, but still a success. But on it we do not intend to stop. We continue to monitor the code review process and solve all technical and even psychological problems faced by employees, so that our process is as productive as possible and at the same time comfortable for the team.

Q & A. What if...?

Q : And what if I think they are following me? What is this timer for?
A : We do not follow specifically for someone, but for the process. In a pull request, there are two sides: the author and the reviewers. If reviewers delay checking, the author suffers. On the other hand, it is also inhumane to tear off the reviewers from work right away. It is necessary to find a balance so that both sides feel comfortable. The timer is needed not to punish someone, but to record all deviations. Most of these deviations happen not through the fault of reviewers, but through the fault of technical problems. The challenge is to fix these problems. That others did not face them. Permanent self-improvement

Q : And what if there are PR complex, with 100500+ lines of code, and there are "change letter". Where's the justice?
A : Yes, it is. When we were working on a standard code review, we just took it along the upper boundary, i.e. The code review of complex PRs should fit into the SLA. But at the same time we have no goal to drive everyone into these boundaries. We always treat with understanding pull-requests, in which there is a lively, useful discussion, despite the knocked down bar in one day.

We have plans for gradations of SLA by storypoints. 1SP - 1h, 3SP - 5h, 5SP - 2d, for example. Fortunately, the Tracker is already capable of that. Simply, we are not yet ready for this - we still have a long way to improve.