Как Мы Сохранили Код-Ревью



Как мы сохранили код-ревью

Я ведущий Java-разработчик в Яндекс.

Деньгах.

Каждый рабочий день в 2018 году меня встречали около 30 запросов на включение, ожидающих рассмотрения, и у меня не было достаточно времени, чтобы разобраться с ними все за день.

В конце лета я ушёл в отпуск, а вернувшись, обнаружил очередь из 50 закрепленных за мной ПР.

Желания их разгребать не было, но это была лишь вершина айсберга, которую я видел своими глазами.

В тот день я решил, что пора что-то менять.

Это история о том, как мы ускорили проверку кода, разгрузили ведущих разработчиков и улучшили инструменты, которыми пользуемся каждый день.



Обзор кода 1.0. Как было раньше?

В Яндекс.

Деньгах code review уже давно стал обязательным этапом разработки, и к нему все уже давно привыкли.

Некоторые понимали, что это так же необходимо, как и тестирование; другие относились к этому как к неизбежному злу, а некоторые сталкивались с ревью кода только как автор запроса на включение, но избегали проверки кода других людей.

Я думаю многие прошли путь последовательно от последнего к первому и это нормально.

Мы использовали Bitbucket для проверки кода с самого начала.

Для каждого репозитория компонентов был указан список из 3-4 рецензентов по умолчанию, которые добавлялись ко всем PR. Обычно этот список составлял и редактировал руководитель отдела, а иногда добавлялись волонтеры, которые сами хотели просмотреть тот или иной компонент. В репозиториях библиотек было немного проще — список рецензентов был одинаковым для всех библиотек, и в него входили старшие разработчики.

В результате почти вся нагрузка легла на рецензентов из числа старших разработчиков, которых постепенно переставало хватать, учитывая рост отдела до 60 человек, увеличение количества репозиториев (60+ компонентов, 100+ библиотек ) и ускорение нашего CI/CD. Помимо большой загруженности и нехватки ресурсов рецензентов, были и другие проблемы:

  • в некоторых компонентах реакции рецензентов можно было ожидать более суток,
  • высокая загруженность назначенных рецензентами по нескольким компонентам,
  • сложно привлечь новых рецензентов, в том числе из-за предыдущего пункта,
  • если главный рецензент болел или был в отпуске, время код-ревью компонентов начинало заметно увеличиваться,
  • назначенные рецензенты не всегда обладали реальным опытом работы с компонентом, и из-за этого страдало качество проверок кода.

Прежде чем решать эти проблемы, нам нужно решить, чего мы обычно ожидаем от проверок кода.



Как провести правильную проверку кода?

Мы определили четыре пункта, которые должны быть в обновленном код-ревью:
  1. Проверка архитектуры решения .

    Вполне очевидная вещь.

    Мы ожидаем этого от старших разработчиков, обладающих опытом в этом компоненте.

  2. Проверка технической реализации , чего мы также ожидаем от представителей старшего и среднего звена, обладающих опытом в этом компоненте.

  3. Передача знаний , который заключается в изучении бизнес-логики и базы кода новичками и новичками посредством проверки кода.

  4. Возможность оценки профессиональных навыков разработчиков .

    Мне бы хотелось, чтобы за каждым разработчиком был закреплен наставник, который оценивает рост, определяет вектор развития, замечает какие-то недостатки, делает замечания и так далее.

    Поэтому наставник тоже должен видеть код своих учеников.

Возможно, кто-то видит другие цели или не согласен с нашими – поделитесь в комментариях.

А пока перейду от формулирования целей к поиску средств их достижения - мы решили, что хотим достичь их всех и (почти) сразу.



Обзор кода 2.0. Как это произошло?

Что мы придумали? Мы начали думать шаг за шагом.

В Яндекс.

Деньгах разработчики работают в командах по направлениям бизнеса, обычно по 2-4 бэкенд-разработчика в команде.

Допустим, я собираюсь открыть запрос на включение, то есть я автор .

У меня есть моя команда , разработчики которого прекрасно знают бизнес-логику того, что я делаю, ведь мы все участвуем в общих проектах, часто синхронизируемся и вообще активно взаимодействуем.

Поэтому я бы хотел сначала добавить их в свои запросы на включение, чтобы они хотя бы знали, что я делаю.

За каждым компонентом в Яндекс.

Деньгах закреплена команда, которая за него отвечает и поддерживает его в работе.

Если я дорабатываю компонент, за который отвечает другая команда, то логичным кажется добавить в рецензенты разработчиков из этой команды — ведь они отвечают за этот компонент и должны следить за качеством его кода.

Но чтобы не перегружать рецензентов, мы возьмем из этой команды только одного случайного человека — считаем, что этого достаточно.

Может случиться так, что команда, ответственная за компонент , не имеет в этом достаточного опыта.

Это происходит, когда в команде появляются новички или им только недавно доверили этот компонент. Однако я знаю, что у нас в компании есть настоящие эксперты по этому репозиторию, и было бы здорово, если бы кто-нибудь из них посмотрел мой код! Конечно, мои знания сложно формализовать, но можно взять историю репозитория и посчитать по количеству PR и статистике code review, кто много модифицировал и/или ревьюировал этот код. Давайте посчитаем метрику экспертности в репозитории, выберем по этой метрике топовых разработчиков и назовем их.

эксперты и мы добавим в рецензенты одного случайного эксперта.

В 2018 году мы ввели в компании институт наставничества, поэтому теперь за каждой командой присматривает один наставник из числа старших разработчиков.

Также у каждого новичка в компании на первых порах есть личный наставник.

Пусть мой наставник посмотрит мой код! Он сможет прийти на помощь в случае возникновения проблем во время проверки кода, а также будет иметь представление о моих слабых и сильных сторонах в технических навыках.



Как мы сохранили код-ревью

Всего к рецензентам каждого пул-реквеста можно добавить пять-шесть человек.

Но на самом деле их обычно немного меньше, потому что в одном человеке могут совмещаться разные роли.

Мой наставник может быть одновременно экспертом, а моя команда может отвечать за компонент. Субъективно кажется, что для пул-реквестов оптимально будет 3-4 рецензента.



Обзор кода 2.0. Что под капотом?

Остаётся только заставить всё это работать.

Здесь помогло то, что все составы нашей команды уже были созданы в отдельной системе, которая при их получении предоставляет REST API. Поэтому после пары недель неторопливой разработки в свободное время родилась первая версия плагина для Bitbucket, которая постепенно дорабатывалась и в течение осени приобрела весь необходимый функционал.



Как работает плагин

Обычно в Bitbucket при создании PR заранее заполняются рецензенты, которые указаны в настройках проекта или репозитория.

Здесь с точки зрения пользователя ничего не изменилось - все рецензенты также предварительно заполняются при открытии этой страницы, разве что добавлено поле с описанием, какой рецензент в какой роли добавлен.

Теперь у нас есть следующие роли для рецензентов:

  • товарищ по команде - член авторской команды PR, легко добавляется благодаря REST API с составами команд,
  • владелец репозитория — случайный член команды, отвечающий за компонент; необходимо было в настройках репозитория предусмотреть возможность выбора ответственной команды,
  • эксперт репозитория - случайный эксперт репозитория; Подробнее об этом я расскажу чуть позже,
  • наставник — наставник команды или новичка, также доступен через REST API из сервиса с составами команд.


Ээксперты по хранилищам

Я расскажу вам немного больше о том, как мы считаем экспертов.

Каждый день плагин проходит по всем репозиториям, просматривает все пулл-реквесты за последний год и вычисляет две простые метрики:

  • количество пул-реквестов, созданных разработчиком,
  • количество PR, которые он рассмотрел и утвердил, требует доработки или уменьшится.

Давайте добавим вес этим метрикам, исходя из того, что с точки зрения экспертизы кода доработка этого кода важнее, чем его рецензирование.

Сначала мы оценили количество созданных пул-реквестов в полтора раза важнее, чем отзывов, а позже увеличили соотношение до трёх к одному.

Суммируем эти метрики, умножаем на их веса и получаем рейтинг разработчика.

Далее мы сортируем всех этих разработчиков по рейтингу, выбираем топ-5 и одновременно отсекаем тех, чей рейтинг ниже порогового, чтобы отсечь случайных прохожих.

И на каждый репозиторий мы обычно набираем от трех до пяти экспертов.

Выше я описал вам подход к выбору рецензентов, который мы выбрали и внедрили, но попутно мы реализовали несколько небольших улучшений, которые сделали процесс код-ревью еще быстрее, удобнее и приятнее.



Запрет на объединение пул-реквестов до тех пор, пока проблема не будет проверена в Jira.

Такая очевидная и нужная вещь, которая, к сожалению, не выходит из коробки.

Мы добавляем в разработку только стабильный код, прошедший не только статические проверки и тесты разработчиков, но и интеграционное тестирование вместе с другими сервисами.

В нашей стране статус такого тестирования отражается только в задаче Jira, и поэтому раньше разработчикам приходилось вручную проверять, проверена ли проблема, чтобы объединить пул-реквест.

Автоматическое объединение запросов на извлечение

Немалую часть своей жизни пул-реквест может находиться в состоянии, когда ничто не мешает его слить, но человек забывает это сделать или делает не сразу.

Яркий пример — ожидание тестирования задачи, без которого мы не сливаем ее в dev. Вот тут-то и пригодится автоматическое слияние, которое работает по простому принципу: если PR можно объединить, то мы это делаем.

Все необходимые условия для слияния проверяются.

Проверяем успешность сборки, уровень тестового покрытия, отсутствие зависимостей библиотеки снапшотов, статус задачи в Jira и наличие всех необходимых согласований.

То есть у нас есть все, чтобы использовать этот функционал и забыть о пиаре с момента прохождения code review и передачи задачи на тестирование (если, конечно, QA не обнаружит с этим проблем).

И мы реализовали это довольно удобным способом: ввели специального бота AutoMergeBot, которого достаточно просто добавить в рецензенты, чтобы он начал мониторить этот пул-реквест и мержить его, когда придет время.



Учет отсутствия рецензента

Если владелец компонента или эксперт находится в отпуске или больничном, он не будет добавлен рецензентом, а его место займет тот, кто находится на рабочем месте.

В качестве бонуса, этот рецензент не будет засыпан горой запросов от других людей по возвращении из отпуска.

Реализовать это было несложно благодаря тому, что все отсутствия сотрудников оформлялись запросами в Jira.

Трудовые книжки рецензента

Некоторые просматривают десять PR в день, другие — пять.

У кого-то уже есть 20 нерассмотренных PR, а у кого-то почти нет. Мы учитываем все это, чтобы более равномерно распределить нагрузку на рецензентов.

Чем больше нагрузка, тем меньше добавляется новых ПР - все просто.



Маркировка размеров PR при создании

На странице создания пул-реквеста автор может выбрать предполагаемый размер пул-реквеста: S, M или L. Это помогает рецензентам оценить примерное время, которое они потратят на ревью кода.

Например, у меня свободно 5 минут, и я понимаю, что могу успеть посмотреть пул-реквест размера S. При этом открывать M или L нет смысла, потому что не успею закончить их, и в следующий раз мне придется начинать с нуля.

В будущем мы хотим учитывать эти атрибуты при расчете PR-статистики.



Маркировка срочного PR

Также при создании PR автор может указать, что задача очень срочная, добавив такой символ к названию PR. Рецензенты сразу это увидят и попытаются просмотреть в первую очередь.

Вроде бы мелочь, но очень полезная и удобная.



Отслеживание начала и окончания проверки кода

Если при улучшении процесса невозможно понять, насколько он улучшился, то нет смысла начинать.

То же самое и с code review — мы можем пытаться улучшить его сколько угодно, но как быть уверенным в положительной динамике без метрик и статистики? В нашем случае это не самая простая задача — Bitbucket и Jira «из коробки» не предоставили возможность отслеживать время code review. У нас была только метрика PR life life, которая нас не совсем устраивала, потому что мы объединяем пул-реквесты только после тестирования задачи, поэтому в эту метрику входили и посторонние показатели.

Jira хранит и позволяет загружать все этапы жизненного цикла задачи, поэтому мы посчитали целесообразным дополнить эти данные двумя дополнительными тегами: временем начала и окончания проверки кода.

Нам просто нужно было научить плагин Bitbucket отправлять эти теги в Jira. Таким образом, Jira была и остается единственной точкой истины для проблемы, и используя этот набор данных, мы можем отделить время проверки кода от времени тестирования проблемы.

Самый тонкий момент здесь — как определить окончание code review? Может быть, это время получения первого одобрения, может быть, последнего, а может быть, это время внесения последних правок автором ПР? У меня нет ответа на этот вопрос; нам просто нужно договориться между собой и выбрать что-то одно или покрыть метриками все три события и отслеживать отклонения.



Отслеживание загрузки рецензента

Еще один полезный показатель — нагрузка рецензента.

Как я писал выше, мы это учитываем при назначении рецензентов на новый пиар, но также публикуем эту информацию, чтобы отслеживать динамику по команде, отделу или компании.

Иногда это помогает обнаружить аномалии и потенциальные проблемы: если видно, что у одного или нескольких человек в команде каждый день появляется 10 и более непросмотренных PR, то есть повод узнать, все ли в порядке.



Просмотр метрик в Grafana

Построение отчетов на данных из Jira полезно, но не совсем удобно, поэтому заодно мы добавили отправку метрик по основным событиям в StatsD, чтобы строить графики по оперативным данным в Grafana. Мы отслеживаем среднее время до первого одобрения, среднее время жизни пиара, а также смотрим на аномальные значения этих метрик, чтобы быстрее находить и решать проблемы.

На момент написания наша панель управления выглядит так:

Как мы сохранили код-ревью

Что вы получили в итоге? К сожалению, мы все сильны задним числом, поэтому мы ввели упомянутые выше метрики code review не до начала изменения самого процесса (сентябрь-октябрь 2018), а уже по ходу дела, поэтому достоверно отслеживать улучшения или ухудшения мы можем только начиная с начала декабря 2018 г.

Что нам удалось заметить? Первое, что бросается в глаза, — это снижение нагрузки на старших разработчиков-рецензентов, и я испытал это на собственном опыте.

Как я уже упоминал, для меня было нормально видеть утром в очереди около 30 ПР, но сейчас это число колеблется между 10 и 15. Статистика по ведомству это подтверждает: начиная с декабря 2018 года максимальное количество ПР, ожидающих отзыв не поднялся выше 15. В среднем мы видим картину, которая говорит о том, что в среднем каждый разработчик ожидает утром 4-5 непросмотренных PR, что кажется вполне адекватным количеством.



Как мы сохранили код-ревью

Что касается актуальности подбора рецензентов и качества code review, то здесь мы можем полагаться только на субъективные показатели.

По опросам коллег, мы действительно стали получать отличный подбор рецензентов, теперь нам больше не приходится никого добавлять вручную, и ни один пиар не остается брошенным и забытым.



Как мы сохранили код-ревью

Если говорить о времени выполнения code review, то рассчитывать статистику по этому показателю пока рано, потому что собирать ее мы начали совсем недавно.

У нас есть только время существования пул-реквестов, которое на самом деле не увеличилось и не упало.

В эту метрику входит время тестирования задачи, поэтому по ней сложно сделать однозначные выводы; кроме того, изменив код-ревью, мы не сделали его длиннее.

Теги: #программирование #ИТ-компании #ИТ-компании #java #Grafana #Идеальный код #ревью кода #BitBucket #Яндекс.

деньги #Jira #statsd

Вместе с данным постом часто просматривают: