Я ведущий Java-разработчик в Яндекс.Деньгах.
Каждое рабочее утро в 2018 году меня встречали около 30 пулл-реквестов, ожидающих ревью, а у меня не хватало времени разобрать их все за день. В конце лета я ушел в отпуск, а когда вернулся, обнаружил очередь из 50 PR, назначенных на меня. Разгребать их не было никакого желания, а ведь это была лишь вершина айсберга, которую я видел своими глазами. В тот день я и решил, что пора что-то изменить.
Это история о том, как мы ускорили код-ревью, разгрузили ведущих разработчиков и улучшили инструменты, которыми пользуемся каждый день.
Код-ревью 1.0. Как было раньше?
В Яндекс.Деньгах код-ревью уже давно стало обязательным этапом разработки, и все к этому давно привыкли. Одни понимали, что это такая же нужная вещь, как тестирование; другие относились к этому как к необходимому злу, а кто-то сталкивался с код-ревью только в роли автора пулл-реквестов, но сторонился ревью чужого кода. Думаю, многие проходили путь последовательного от последнего к первому, и это нормально.
Для код-ревью мы с самого начала использовали Bitbucket. Для каждого репозитория компонента задавался список из 3-4 ревьюеров по умолчанию, которые добавлялись во все PR. Обычно этот список составлялся и редактировался начальником отдела, а иногда туда добавлялись добровольцы, которые сами хотели ревьюить тот или иной компонент. В репозиториях библиотек было чуть проще – список ревьюеров был единым для всех библиотек, и туда входили старшие разработчики.
В итоге почти вся нагрузка ложилась на ревьюеров из числа старших разработчиков, которых постепенно переставало хватать, с учетом роста отдела до 60 человек, увеличения количества репозиториев (60+ компонентов, 100+ библиотек) и ускорения нашего CI/CD.
Кроме большой нагрузки и нехватки ресурса ревьюеров, были и другие проблемы:
- в некоторых компонентах можно было ожидать реакции ревьюеров более суток,
- высокая загруженность тех, кто назначен ревьюером в нескольких компонентах,
- тяжело привлекать новых ревьюеров, в том числе из-за предыдущего пункта,
- если основной ревьюер заболел или в отпуске, время код-ревью компонентов начинало заметно увеличиваться,
- назначенные ревьюеры не всегда имели реальную экспертизу в компоненте, из-за этого страдало качество код-ревью.
Перед тем как решать эти проблемы, нужно определиться с тем, чего мы вообще ждем от код-ревью.
Правильное код-ревью — это как?
Мы выделили четыре пункта, которые должны быть в обновленном код-ревью:
- Проверка архитектуры решения. Довольно очевидная вещь. Ожидаем это от старших разработчиков с экспертизой в данном компоненте.
- Проверка технической реализации, которую ожидаем также от старших и мидлов с экспертизой в данном компоненте.
- Передача знаний, которая заключается в изучении бизнес-логики и кодовой базы новичками и джунами посредством код-ревью.
- Возможность оценки hard skills разработчиков. Хочется, чтобы за каждым разработчиком был закреплен наставник, который оценивает рост, определяет вектор развития, подмечает какие-то недостатки, делает замечания и так далее. Поэтому наставник также должен видеть код своих подопечных.
Возможно, кто-то видит другие цели или не согласен с нашими — делитесь в комментариях. А я пока перейду от формулирования целей к поиску средств их достижения – мы решили, что хотим достичь их все и (почти) сразу.
Код-ревью 2.0. Как стало?
Что же мы придумали? Мы стали рассуждать пошагово.
В Яндекс.Деньгах разработчики работают в командах по бизнес-направлениям, обычно по 2-4 бэкенд-разработчика в команде.
Допустим, я собираюсь открыть пулл-реквест, то есть я его автор. У меня есть моя команда, разработчики которой хорошо знают бизнес-логику того, что я делаю, потому что мы все участвуем в общих проектах, часто синхронизируемся и вообще активно взаимодействуем. Поэтому хочется добавлять их в мои пулл-реквесты в первую очередь, чтобы они как минимум были в курсе того, что я делаю.
За каждым компонентом в Яндекс.Деньгах закреплена команда, которая за него отвечает и сопровождает на продакшене.
Если я дорабатываю компонент, за который отвечает другая команда, то кажется логичным добавлять разработчиков из этой команды в ревьюеры – они ведь отвечают за этот компонент и должны следить за качеством его кода. Но, чтобы не перегружать ревьюеров, возьмем только одного случайного человека из этой команды — считаем, что этого достаточно.
Может случиться так, что команда, отвечающая за компонент, не владеет в нём достаточной экспертизой. Такое бывает, когда в команде появляются новички или им вверили этот компонент только недавно. Однако я знаю, что у нас в компании есть реальные эксперты по этому репозиторию, и было бы классно, чтобы кто-то из них посмотрел мой код! Конечно, моё знание сложно формализовать, но ведь можно взять историю репозитория и посчитать по количеству PR и статистике код-ревью, кто много дорабатывал и/или много ревьюил этот код. Посчитаем метрику экспертизы в репозитории, выделим топ разработчиков по этой метрике, назовем их экспертами и будем добавлять одного случайного эксперта в ревьюеры.
В 2018 году мы в компании ввели институт наставничества, поэтому сейчас за каждой командой приглядывает один наставник из числа старших разработчиков. Также у каждого новичка в компании на первых порах есть персональный наставник.
Пускай же мой код смотрит мой наставник! Он сможет прийти на помощь в случае проблем на код-ревью, а также будет иметь представление о моих слабых и сильных сторонах в технических навыках.
Итого в ревьюеры каждого пулл-реквеста может добавиться пять-шесть человек. Но на самом деле обычно их чуть меньше, потому что разные роли могут совмещаться в одном человеке. Мой наставник может одновременно быть и экспертом, а моя команда может быть ответственной за компонент. Субъективно кажется, что 3-4 ревьюера было бы оптимально для пулл-реквестов.
Код-ревью 2.0. Что под капотом?
Дело за малым: заставить всё это работать. Помогло здесь то, что все наши составы команд уже были заведены в отдельной системе, которая предоставляет REST API по их получению. Поэтому после пары недель неторопливой разработки в свободное время родилась первая версия плагина для Bitbucket, который постепенно дорабатывался и в течение осени обзавелся всем необходимым набором функциональности.
Как работает плагин
Обычно в Bitbucket при создании PR предзаполняются ревьюеры, которые заданы в настройках проекта или репозитория. Тут с точки зрения пользователя ничего не изменилось — все ревьюеры также предзаполняются при открытии этой страницы, разве что добавилось поле с описанием, какой ревьюер в какой роли добавился. А роли ревьюеров у нас появились следующие:
- teammate — член команды автора PR, легко добавляется благодаря REST API с составами команд,
- repository owner — случайный член команды, которая отвечает за компонент; понадобилось в настройках репозитория дать возможность выбирать ответственную команду,
- repository expert — случайный эксперт по репозиторию; об этом подробнее расскажу чуть позже,
- mentor — наставник команды или новичка, также доступен по REST API из сервиса с составами команд.
Эксперты по репозиториям
Чуть подробнее расскажу о том, как мы считаем экспертов. Ежесуточно плагин проходит по всем репозиториям, смотрит все пулл-реквесты за последний год и считает две простые метрики:
- количество пулл-реквестов, созданных разработчиком,
- количество PR, которые он отревьюил и поставил approve, needs work или decline.
Добавим веса к этим метрикам исходя из того, что с точки зрения экспертизы в коде, доработка этого кода важнее, чем ревью. Сначала мы оценили количество созданных пулл-реквестов в полтора раза важнее ревью, а позже увеличили соотношение до трех к одному. Суммируем этим метрики, умноженные на их веса, и получаем рейтинг разработчика.
Далее сортируем всех этих разработчиков по рейтингу, выбираем топ-5, попутно отсекаем тех, чей рейтинг ниже порогового, чтобы отсечь случайных прохожих. И получаем обычно от трех до пяти экспертов по каждому репозиторию.
Выше я описал вам подход к подбору ревьюеров, который мы выбрали и реализовали, но попутно мы реализовали сразу несколько небольших улучшений, которые сделали процесс код-ревью еще быстрее, удобнее и приятнее.
Запрет merge пулл-реквеста, пока не проверена задача в Jira
Такая очевидная и нужная вещь, которая, к сожалению, не идет из коробки. У нас в dev попадает только стабильный код, который прошел не только статические проверки и тесты разработчиков, но и интеграционное тестирование вместе с другими сервисами. Статус такого тестирования у нас отражается только в задаче Jira, и поэтому ранее разработчикам приходилось вручную смотреть, проверена ли задача, чтобы вмержить пулл-реквест.
Автоматический merge пулл-реквестов
Немалую часть своей жизни пулл-реквест может провести в состоянии, когда уже ничего не мешает его вмержить, но человек забывает это сделать или делает не сразу. Яркий пример — ожидание тестирования задачи, без которого мы не мержим ее в dev. Тут и пригождается автоматический merge, который работает по простому принципу: если PR можно вмержить, то мы это делаем.
Все необходимые условия для merge покрыты проверками. Мы проверяем успешность сборки, уровень покрытия тестами, отсутствие snapshot-зависимостей библиотек, статус задачи в Jira и наличие всех необходимых аппрувов. То есть у нас есть всё, чтобы использовать такую функциональность и забывать о PR с момента прохождения код-ревью и передачи задачи в тестирование (если конечно QA в ней не обнаружит проблем).
А реализовали мы это довольно удобным образом: ввели специального бота AutoMergeBot, которого просто нужно добавить в ревьюеры, чтобы он начал следить за этим пулл-реквестом и вмержил его, когда придет время.
Учет отсутствий ревьюеров
Если владелец компонента или эксперт в отпуске или на больничном, он не будет добавляться ревьюером, а его место займет тот, кто находится на рабочем месте. В качестве бонуса на этого ревьюера по выходе из отпуска не свалится гора чужих пулл-реквестов. Реализовать это было нетрудно благодаря тому, что все отсутствия сотрудников у нас заведены заявками в Jira.
Учет занятости ревьюеров
Кто-то ревьюит десять PR в день, а кто-то пять. У кого-то уже накопилось 20 непросмотренных PR, а у кого-то их почти нет. Мы всё это учитываем, чтобы более равномерно распределять нагрузку на ревьюеров. Чем больше нагрузки, тем меньше его добавляют в новые PR — всё просто.
Маркировка размеров PR при создании
На странице создания пулл-реквеста автор может выбрать его ориентировочный размер: S, M или L. Это помогает ревьюерам оценить примерное время, которое они потратят на код-ревью. Например, у меня есть свободные 5 минут, и я понимаю, что смогу успеть посмотреть пулл-реквест размера S. При этом нет смысла открывать M или L, потому что я не успею их досмотреть и в следующий раз придется начинать с чистого листа.
В будущем мы хотим учитывать эти атрибуты при подсчете статистики по PR.
Маркировка срочных PR
Также при создании PR автор может указать, что задача очень срочная, добавив такой символ в название PR. Его сразу увидят ревьюеры и постараются посмотреть в первую очередь. Вроде мелочь, а очень полезная и удобная.
Трекинг начала и окончания код-ревью
Если при улучшении процесса невозможно понять, насколько он улучшился, то и начинать нет смысла.
Так и с код-ревью — мы сколько угодно можем пытаться его улучшить, но как мы будем уверены в позитивной динамике без метрик и статистики? В нашем случае это не самая простая задача — из коробки Bitbucket и Jira не давали возможности отслеживать время код-ревью. На вооружении была только метрика времени жизни PR, которая не совсем нас устраивала, ведь мы мержим пулл-реквесты только после окончания тестирования задачи, поэтому в данную метрику замешивались и посторонние показатели.
Jira хранит и позволяет выгружать все контрольные точки жизненного цикла задачи, поэтому мы посчитали правильным обогатить эти данные двумя дополнительными метками: временем начала и окончания код-ревью. Нужно было просто научить плагин для Bitbucket пушить эти метку в Jira. Таким образом, Jira как была, так и остается single point of truth для задачи, и по этому набору данных мы можем отделить время код-ревью от времени тестирования задачи.
Самый тонкий момент здесь в том, как определить момент окончания код-ревью? Может, это время получения первого аппрува, может, последнего, а может, это время внесения последних правок автором PR? У меня нет ответа на этот вопрос, тут просто надо договориться между собой и выбрать что-то одно либо покрывать метриками все три события и следить за девиациями.
Трекинг загрузки ревьюеров
Еще одна полезная метрика — загруженность ревьюеров. Как я писал выше, мы учитываем её при назначении ревьюеров в новые PR, но также мы публикуем эту информацию для того, чтобы следить за динамикой по командам, отделу или компании. Иногда это помогает обнаруживать аномалии и потенциальные проблемы: если видно, что у одного или нескольких человек в команде каждый день висит по 10 и более непросмотренных PR, значит, есть повод узнать, всё ли в порядке.
Просмотр метрик в Grafana
Строить отчеты по данным из Jira полезно, но не совсем удобно, поэтому мы одновременно с этим добавили отправку метрик по основным событиям в StatsD, чтобы строить графики по оперативным данным в Grafana. Мы следим за средним временем до первого аппрува, средним временем жизни PR, а также смотрим аномальные значения по этим метрикам, чтобы оперативно находить и решать проблемы.
На момент написания статьи наш дашборд выглядит так:
Что получили в итоге?
К сожалению, все мы сильны задним умом, поэтому упомянутые выше метрики код-ревью мы ввели не до начала изменения самого процесса (сентябрь-октябрь 2018), а уже по ходу, поэтому достоверно отслеживать улучшения или ухудшения мы можем только начиная с начала декабря 2018. Что же мы успели заметить?
Первым делом бросается в глаза снижение нагрузки на ревьюеров из числа старших разработчиков, и я почувствовал это на собственном примере. Как я уже упоминал, для меня было нормой видеть с утра около 30 PR в очереди, сейчас же это число колеблется между 10 и 15. Статистика по отделу подтверждает это: начиная с декабря 2018 максимальное количество PR, ожидающих ревью, ни у кого не поднимается выше 15. В среднем же мы наблюдаем картину, которая говорит о том, что в среднем каждого разработчика с утра ожидает 4-5 непросмотренных PR, что видится вполне адекватным числом.
Что касается релевантности подбора ревьюеров и качества проведения код-ревью, тут мы можем опираться только на субъективные показатели. По опросам коллег, мы и правда стали получать отличный подбор ревьюеров, никого теперь не приходится добавлять вручную, и никакие PR не остаются брошенными и забытыми.
Если же говорить о времени прохождения код-ревью, то статистику по этому показателю пока рано подсчитывать, потому что мы начали её собирать совсем недавно. В нашем распоряжении есть только время жизни пулл-реквестов, которое на самом деле не выросло и не упало. Эта метрика включает в себя время тестирования задачи, поэтому однозначные выводы по ней сложно сделать, кроме того, изменив код-ревью, мы не сделали его более долгим.
Автор: Валерий Чуркин
