Лучшие практики RuStore: правила хорошего Code Review для Android

от автора

Привет, я Михаил Емельянов, руководитель Android-направления в RuStore. Над стором трудится большая команда разработчиков, проект регулярно дорабатывается, а количество новых строк кода неизменно увеличивается. 

Немного истории

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

В этой статье расскажем, как мы построили процесс Code Review в RuStore, какую проблему решали, поделимся нашими практиками и сделаем выводы.

Мотивация 

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

Мы решили пойти другим путём. Когда RuStore появился как продукт и набрал первый миллион пользователей, Android-команда состояла из 10 человек. В такой команде процесс Code Review проходил быстро: в среднем ревью MR’ов (merge-request) с правками занимало не более 3 дней. И этого было достаточно. Но!

Затем назрела необходимость расширяться и увеличить команду за короткий период в 3 раза. Уже на тот момент мы понимали, что текущий процесс ревью создаст для нас большие проблемы: MTM (Mean time merge) MR’ов начнёт увеличиваться, следовательно ТТМ (Time To Market) задач – тоже.

Учитывая все эти факторы, мы принялись заново выстраивать процесс Code Review, решая эти проблемы системно.

Время жизни MR’а на 4Q 2022 (Команда: 10 человек, 1 ревьювер на MR)

Правила Code Review

Начали мы с того, что описали элементарные правила проверки кода, чтобы инженеру было проще быстро и качественно провести ревью любого MR’а своего коллеги. 

На MR можно смотреть бесконечно долго, но это не приведёт код к требуемому качеству. А можно посмотреть прям быстро, особенно если сильно просит ваш коллега по команде, но и это тоже не решает проблему! Поэтому лучшей практикой будет всегда честно отвечать на простые вопросы:

Критерии

Вопросы

Решение задачи

Решает ли код именно описанную задачу и только её? Указано ли это в описании MR?

Дизайн кода

Код спроектирован по нашим требованиям к архитектуре? (например, правильно выделены модели и бизнес-логика, не нарушена ли слоенность, есть ли проблемы с SRP и т. д.)

Функциональность

Поведение кода ожидаемо автором? Улучшает ли код пользовательскую историю?

Сложность

Может ли код быть написан проще? Сможет ли другой разработчик легко понять и использовать данный код, если это будет необходимо в задачах?

Unit-тесты

Написал? Проверяют ли тесты логику кода?

Интеграция API

Правильно ли используется SDK/API при интеграции?

Наименования

Будут ли выбранные названия переменных, методов, классов понятны для всех?

Комментарии

Есть ли комментарии в сложных местах? Все ли они понятны и полезны?

Стиль

Удовлетворяет ли код нашему принятому код стайлу?

(обычно автоматизируется автоформатированием или проверками статического анализатора)

  • Время старта ревью рассчитывается с момента создания MR и выборки для него ревьюверов (им приходят уведомления).

  • Ревьюверы накидывают комменты (Response time ~ 4 ч).

  • Автор правит, далее процесс повторяется (Reaction time ~ 6 ч).

  • Ревьюверы, после проверки изменений, ставят аппрувы (лайки).

  • MR сливается в dev или проходит отдельную стадию тестирования.

На всё это рекомендуется тратить не больше 24 ч, из которых на комменты, замечания и правки MR должно уходить не больше 10 ч (response time = 4 ч, reaction time = 6 ч). Остальное время распределяется в рамках личного эффективного времени инженера, закладывая также лаг, связанный с разными тайм зонами команд. Если MR относится к рутовой ветке фичи при классическом Git Flow, то ко времени влития MR’а добавляется время тестирования этой ветки.

Как выбрать владельцев кода (Code Owners)?

Владение кодом – механизм разделения кода между командами, который позволяет назначать ответственных (Code Owners) за отдельные части кода в проекте.

По мере роста проекта и команды разработчиков нам стало очевидно, что там, где за результат отвечают все, в итоге, не отвечает никто. Например, если в какой-то модуль проекта как через проходную «ходит» вся команда, то при появлении техдолга будет непросто найти ответственного. Это побудило нас настроить владельцев кода (Code Owners) в репозитории. Теперь за любым кодом в проекте закреплена конкретная команда ответственных ревьюеров – известное решение, но при построении процесса ревью играет ключевое значение.

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

Добавление Code Owners в качестве ревьюеров позволяет:

  • улучшить качество и ускорить Code Review;

  • обеспечить полную осведомленность обо всех изменениях в модулях для ответственных ревьюеров; 

  • упросить выбор ревьюеров при создании MR (GitLab отображает подсказки).

Валидация README

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

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

Поэтому в отдельную стадию мы выделили проверку корректности файла README.md в CI.

Реализовано это в виде ValidatorConventionPlugin и task – validateReadme.

В будущем мы планируем добавлять в этот плагин и другие проверки, поэтому для общей валидации предусмотрена общая task – validate.

Локально проверяем так:

./gradlew validate

или

./gradlew validateRead

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

Gradle-task для генерации Code Owners по содержимому README.md

В App добавили gradle-task – generateCodeowners – для генерации файла ./gitlab/CODEOWNERS по содержимому файлов README.md.

# [Имя модуля]  # # Owner  [Команда-владелец кода]  # # Description  [Описание модуля]

Эту задачу удобно использовать, когда в результате реинжиниринга вы создаёте/удаляете новые модули, при этом нужно только правильно заполнить команду в README.md, запустить task и закоммитить изменения.

Мы также добавили валидацию актуальности в MR.

Что в итоге

Мы выделили несколько важных пунктов, которые стоит учитывать при организации самого процесса Code Review, а именно:

  1. Определите критерии и требования к ревью кода.

  2. Назначьте необходимое количество ревьюеров, чтобы процесс Code Review проходил более эффективно и не отнимал ТТМ.

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

  4. Добавьте удобное описание: Что делает данный код? За что отвечает?

Соблюдая эти правила, мы смогли эффективно проводить Code Review, даже когда команда увеличилась в 3 раза, не снижая при этом темпы разработки.

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

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

P.S. Наши практики нельзя назвать универсальными, в каждой команде процесс ревью может быть организован по-своему. Мы поделились с вами своим подходом, но не претендуем на то, что он идеален для всех ? Какие практики используете вы? Будем рады почитать в комментариях.

Спасибо!


ссылка на оригинал статьи https://habr.com/ru/articles/790044/


Комментарии

Добавить комментарий

Ваш адрес email не будет опубликован. Обязательные поля помечены *