Человек – главная ценность любого предприятия. Успех всего дела зависит от того, как люди коммуницируют между собой, как вместе добиваются поставленных целей, от командной работы. Постоянное оттачивание навыков, процессов и инструментов позволяет работать эффективней.
Мы в Маркете работаем над тем, чтобы как можно быстрее доставлять новые возможности нашим пользователям. Для этого мы постоянно изучаем наши процессы и оптимизируем все этапы работы над задачей. Сегодня мы расскажем читателям Хабра об оптимизации одного из них, а именно процесса код-ревью.
Для начала надо представить, какой флоу в разработке у нас принят:
То же самое, только по пунктам и словами:
- Разработчик берет таску.
- Делает ее.
- Заливает в Github и открывает PR.
- Проходит код-ревью.
- Собирает демо-стенд и отдает тестировщику.
- Тестировщик проверяет демо-стенд.
- Проверенную таску собирают в релиз.
- Тестирование в релизе.
- Выкладка на прод.
- Финальное тестирование на бою.
По зонам ответственности таска делится на следующие шаги:
1-5 — ответственность на разработчике;
6-7 — ответственность на тестировщике;
8-10 — ответственность на релиз-мастере.
Итак, начали анализировать, что же занимает у нас больше всего времени. Благо есть внутренние инструменты анализа. Все делали ставку на то, что дольше всего будет занимать, само собой, сам процесс разработки (статус «В работе»)… и так и оказалось. Но один момент нас удивил больше всех. Средняя продолжительность ревью — две недели!
Действие 1. Разбор PR
Начав изучать пулл-реквесты, сразу же столкнулись с одним очень интересным фактом. Оказалось, что у нас есть огромное кладбище незакрытых пулл-реквестов. Большинство авторов этих PR уже не работали в компании, но их наследие всё еще было с нами. Кто никогда не видел динозавров, то вот они:
Эти пулл-реквесты добавляли шум и мешали правильному анализу времени код-ревью. Со спокойной душой мы их позакрывали. Оставалось только пересчитать время. А оно по-прежнему было в районе 2-3 дней. Нехорошо.
Действие 2. Разборка с ревьюшницей
Ревьюшница — разработанная в Яндексе система, которая призывает в PR двух рандомных людей с экспертизой в изменяемом коде и с учетом отпусков и отсутствий. Разбор недельных PRов выявил группу лиц, которые постоянно затягивают код-ревью. Опросив данных коллег, мы нашли еще одну проблему в нашем процессе. Коллеги жаловались, что им приходит слишком много пулл-реквестов за день от ревьюшницы, и у них просто не хватает времени на их основную работу.
Это не сходилось с нашими показателями: один-два PR в день на человека. Исследование привело в Github, где у нас заведена отдельная команда ревьюеров. Эта команда не обновлялась несколько лет. С тех пор количество сотрудников увеличилось в два раза, но никто из новичков в команду ревьюеров не входил. Иначе говоря, половина команды вообще не участвовала в код-ревью! Исправили это досадное недоразумение.
Действие 3. Расширение инструментов
На этом этапе мы пытались упростить и без того простую, как мы думали, жизнь ревьюеров. В арсенале фронтендеров были следующие инструменты:
- кодстайл-чекеры;
- прогон тестов;
- различные проверки-нероняйки в самом PR;
- ревьюшница;
- оповещения на почту и в таск-трекер.
Казалось бы, всё есть. Бери и ревьюй!
Первое, что оказалось не так: разный процесс код-ревью в разных репозиториях. Унифицировали и заодно докинули проставление лейблов для удобного поиска PRов через интерфейс Github.
Второе, что заметили: почта — не лучший инструмент для быстрого информирования о статусе код-ревью. Многие, чтобы не отвлекаться от работы, разбирает свою почту несколько раз за день. Совсем другое дело мессенджеры. Реакция на сообщения в мессенджерах куда выше. И было решено подключить бота к нашему мессенджеру. Бот шлёт оповещения о статусе код-ревью как для автора пулл-реквеста, так и призывает людей поревьюить. Очень удобно.
Действие 4. Первый SLA
Параллельно действиям 2 и 3 мы начали плотно работать с сотрудниками и объяснять, что код-ревью неотделим от выполнения самой задачи. Объясняли, что доведение до «Проверено» — это ответственность разработчика. Причем ответственность не только перед коллегами, но и перед пользователями! Быстрая доставка фичи до прода — вот чего хотелось достичь. И команда с пониманием отнеслась к процессу улучшения.
На этом этапе зародилось первое представление об идеальном код-ревью. Конечно, всем хочется, чтобы оно проходило за 5 минут, но это возможно не всегда. Взяв в расчет, что у нас мультирегиональная команда (с разницей во времени в четыре часа), мы договорились, что нашим SLA будет один день, т.е. 24 часа. Огласили всем сотрудникам этот SLA и, потирая руки, стали ждать результатов.
Но ситуация так и не изменилась. В лучшие недели половина пулл-реквестов укладывалось в 1 день. Остальные так и вылезали за него.
У нас был небольшой скрипт, который оценивал время код-ревью на PR. Им мы и стали грепать на ежедневной основе все PRы в поисках "отстающих". Почти каждый день находилась пачка таких.
На их разбор уходило много времени. Чаще всего сам скрипт нечестно обсчитывал время ревью. Он не учитывал, что некоторые PRы создавали в нерабочее время (да, у нас некоторые смелые-умелые любят поработать часок-другой ночью, приходите к нам работать!). Всё это говорило нам о том, что наши инструменты мониторинга пулл-реквестов не самые эффективные, так как нечестные. Придётся найти новые инструменты.
Действие 5. Трекер SLA
Помощь пришла откуда не ждали. Наши коллеги из Трекера анонсировали удивительную вещь: теперь можно установить SLA в самом Трекере. Причем настроить его можно совершенно разнообразно. Некий таймер включается по какому-то событию в таске. По какому-то событию может вставать на паузу. И по какому-то событию останавливаться. Да это то, что нам надо!
Сразу полезли детально изучать документацию (кстати, вот она) и настраивать наши очереди (их несколько, так как несколько репозиториев). Настроили, чтобы таймер включался при переходе в статус "Need code review" и завершался при переходе в любой другой статус, кроме "Есть замечания" – при нём таймер вставал на паузу, т.е. не сбрасывал своё время.
Ещё оказалось классно то, что таймер учитывал рабочее время и календарь! Мы выставили, что на код-ревью отводится 9h, т.е. один рабочий день. При этом оповещение срабатывает через 2h после старта, или если сорвали девятичасовой срок. Получился своеобразный честный мониторинг. Поначалу мы включили таймер ради эксперимента, собрали пачку багов и отрепортили в Трекер.
Стоит отметить ещё то, что таймер включался только для тасков, которые были созданы уже после внедрения самого таймера. Поэтому мгновенного эффекта увидеть не удалось. Но уже на этом этапе началась положительная динамика. Эффект мы получили уже спустя месяц, когда поток новых тикетов с таймером стал вытеснять старые. Было заметно, что примерное время код-ревью концентрировалось в районе сообщений-напоминалок, т.е. отметок 2h и 9h.
На момент написания этой статьи у нас 415 тасков, в которых таймер включился. Из них 29 вышли за девятичасовую границу. Хотя, если внимательно приглядеться, то встречаются и такие таски, код-ревью которых завершалось в течение следующего часа. Если отбросить и их, то остаётся 17 тасков. А это примерно 4.1%!
Итог. Самурай постоянно точит свой меч
Оглядываясь назад и вспоминая все наши действия, можно сделать один вывод — все средства хороши. Все наши шаги привели к желаемому результату: более 92% пулл-реквестов стали удовлетворять нашему SLA! Всё меньше сорванных тасков, всё быстрее проходит код-ревью. Потихоньку-помаленьку 75% код-ревью стало укладываться в 5 часов! И динамика по улучшению до сих пор положительная. Помимо числовых показателей мы стали получать еще положительные отзывы от смежников. Еще больше радует тот факт, как команда отнеслась ко всему этому процессу. Даже такой процесс, как ускорение код-ревью, еще больше сплотил команду. Каждый участник стал понимать ответственность, которую он несёт перед другими, как быстрое код-ревью позволяет еще быстрее приносить пользу нашим пользователям.
Конечно, 9h это не предел мечтаний, но всё-таки успех. Но на нём мы не намерены останавливаться. Мы продолжаем отслеживать процесс код-ревью и решать все технические и даже психологические проблемы, с которыми сталкиваются сотрудники, чтобы наш процесс был максимально продуктивным и в то же время комфортным для команды.
Q&A. А что, если...?
Q: А что, если я думаю, что за мной следят? К чему этот таймер?
A: Следим не конкретно за кем-то, а за процессом. В пулл-реквесте две стороны: автор и ревьюеры. Если ревьюеры затягивают с проверкой, то страдает автор. С другой стороны отрывать ревьюеров от работы прямо так сразу тоже негуманно. Нужно найти баланс, чтобы комфортно было обеим сторонам. Таймер нужен не для того, чтобы кого-то наказать, а для того, чтобы фиксировать все отклонения. Большинство таких отклонений случается не по вине ревьюеров, а по вине технических проблем. Задача в том, чтобы эти проблемы устранять. Чтобы другие с ними не столкнулись. Постоянное самосовершенствование
Q: А что, если бывают PR сложные, по 100500+ строк кода, а бывают "букву поменять". Где справедливость?
A: Да, это действительно так. Когда мы прорабатывали стандарт код-ревью, то как раз взяли по верхней границе, т.е. код-ревью сложных PR должно укладываться в SLA. Но при этом у нас нет цели загнать всех в эти границы. Мы всегда с пониманием относимся к пулл-реквестам, в которых идёт живое, полезное обсуждение, невзирая на сбитую планку в один день.
У нас в планах есть градации SLA по storypoints. 1SP — 1h, 3SP — 5h, 5SP — 2d, к примеру. К счастью, Трекер уже способен на такое. Просто мы к такому еще не готовы – нам предстоит еще долгий путь совершенствования.
