Привет, я Михаил Емельянов, руководитель 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 при интеграции? |
Наименования | Будут ли выбранные названия переменных, методов, классов понятны для всех? |
Комментарии | Есть ли комментарии в сложных местах? Все ли они понятны и полезны? |
Стиль | Удовлетворяет ли код нашему принятому код стайлу? (обычно автоматизируется автоформатированием или проверками статического анализатора) |
Процесс Code Review не должен использоваться для выяснения (не)профессиональных отношений или повышения своего ЧСВ.
Как выбирать ревьюеров?
Участники Code Review (ревьюеры) должны быть хорошо знакомы с кодом, в который были внесены изменения. Это позволит качественно достичь цели самого ревью с минимальными временными затратами. Для этой роли лучше всего подходят владельцы кода (Code Owners) (о том, как и для чего нужно назначать владельцев кода, рассказываем ниже).
Сколько нужно ревьюеров?
Мы используем простую формулу расчета, которую легко понимать, и так же легко автоматизировать:
2R = 1L + 1D
или
3R = 1L + 1D + 1QA
где:
R - reviewer-ы(2-3)
L - техлид (1)
D - инженер (НЕ техлид) (1)
QA - тестировщик (1)Итак, в каждом MR у нас всегда участвуют 2-3 ревьювера. Этого количества вполне достаточно для нашего ТТМ и обеспечения качества кода.
Правило аппруверов, которое позволяет настроить минимальное необходимое количество ревьюеров, должно быть автоматизировано.
Лишние правила можно удалить, например, если ваша задача не требует тестирования и снабжена меткой skip-test.
Дополнительные ревьюеры
Если для проверки каких-то изменений вам понадобятся компетенции ещё одного коллеги, линканите его в описании или в комментарии под нужным куском кода. Можно также добавить его в ревьюеры, чтобы сразу было видно, что он проверил код. Убедитесь, что коллега в курсе, что от него нужна проверка не всего MR, а только конкретного куска (чтобы он не тратил время на весь MR и не затягивал время Code Review)!
Сколько времени закладывать на ревью?
Время на Code Review мы рассчитываем по следующему графику:
Время старта ревью рассчитывается с момента создания 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, а именно:
Определите критерии и требования к ревью кода.
Назначьте необходимое количество ревьюеров, чтобы процесс Code Review проходил более эффективно и не отнимал ТТМ.
Закрепите ответственных за код, чтобы уменьшить время погружения в процессе ревью.
Добавьте удобное описание: Что делает данный код? За что отвечает?
Соблюдая эти правила, мы смогли эффективно проводить Code Review, даже когда команда увеличилась в 3 раза, не снижая при этом темпы разработки.
Сейчас мы по-прежнему укладываемся в 3-4 дня MTM на MR с учетом ревью, при этом, у нас значительно увеличился объём кода и количество MR.
Стоит отметить, что мы пресекли попадание некачественного кода и решений, что случалось в рамках старого процесса ревью. И это действительно большое достижение для такой огромной команды!
P.S. Наши практики нельзя назвать универсальными, в каждой команде процесс ревью может быть организован по-своему. Мы поделились с вами своим подходом, но не претендуем на то, что он идеален для всех ? Какие практики используете вы? Будем рады почитать в комментариях.
Спасибо!
