Как отбить желание смотреть ваш пул-реквест? - Вот отличный пример из опен-сор проекта PocketBase: непосредственно ПР.
Конечно, это гипербола, но через крайности объяснять суть всегда нагляднее.
Что мы имеем: изменено 147 файлов, сделан 1 коммит.
Думаю, никто не удивится ответу автора проекта: “Thank you for spending your time on this but this type of changes are not really welcomed as I don’t really see much point of reviewing 140+ files.”
Занавес…
Я не раз говорил о важности маленьких шагов: как они помогают достигать целей и побеждать прокрастинацию.
Но сейчас речь пойдет про то, какие изменения за один раз мы деплоим на прод.
В начале года немного рефлексировал на тему атомарности коммитов. Повторюсь: делайте их чаще, делайте их понятными - это значительно упростит работу вам и вашим коллегам.
Никто не спорит: удобно выбрать все изменения и в одном коммите под знаменем “рефактор” отправить на удаленный репозиторий, но это очень темная дорожка и вам не нужно по ней идти.
Если с коммитами все понятно, то с ПР все гораздо интереснее, тут возможны варианты.
Поменялось ли что-нибудь, если изменения в 147 файлах шли не одним, а десятком коммитов в одном ПР-е?
- Не думаю.
Когда речь идет про код-ревью - красиво оформленный пул-реквест - это отражение вашей инженерной культуры. Он показывает не только вашу аккуратность в работе, но и уважение к тем, кто будет его смотреть.
Представьте, что вы владелец горизонтального сервиса. У вас полно своих задач, которые нужно успевать делать, но также на вас есть ответственность за стабильность своего сервиса перед клиентами и вы в ответе за то, чтобы вертикальные команды как можно быстрее могли релизить свои изменения независимо от вас, чтобы бизнес мог развиваться, а TTM новых фичей был маленьким.
И тут вам прилетает уведомление: у вас 3 новых пул-реквеста:
ПР 1:
- “Добавляю новые правила валидации, изменяю логику для стораджа и делаю небольшой рефактор”
- 16 коммитов (атомарные, понятные)
- изменено 5 файлов, добавлено 2 (1 для тестов на правила валидации)
ПР 2:
- “Прокидываю клиент для метрик и начинаю писать технические метрики для фичи Отображение на карте”
- 5 коммитов (атомарные, понятные)
- изменено 3 файла, добавлен 1 для тестов
ПР 3:
- “В соответствии с задачей VRT-2231 добавляю новую логику”
- 3 коммита (неатомарные)
- изменено 2 файла, добавлен 1 (без тестов)
Какой из этих вариантов посмотрите в первую очередь? Я бы смотрел так: 2 -> 1 -> 3
Объясню:
- Сперва описание. Если оно есть и понятное - уже очень хорошо. Но, если на первой же строке вас за дополнительным контекстом отсылают в Джиру, то это тревожный звонок - вероятно, ревью затянется.
- Если изменения составляют инкремент, как история с метриками во втором ПР, то это явный кандидат на приоритетное ревью. С ним можно быстро разобраться и пойти дальше, позволив понятным изменения быстрее оказаться в проде.
- С ПР 1 и ПР 3 - интереснее. Начну с первого, так как есть большой шанс, что здесь потребуются доработки и я быстрее смогу дать фидбек ребятам на исправление, так как нужны серьезные причины, чтобы столько всего предлагать в одном ПР-е (почему не разбить на 3?).
- ПР 3 уходит напоследок, так для погружения в контекст потребуется гораздо больше ресурсов (из описания вообще ничего не понятно).
Из опыта
Конечно, на практике не все идеально и ожидание код-ревью действительно может стать бутылочным горлышком.
Признаюсь, что и мне приходилось делать один большой ПР, так за спринт не всегда есть возможность успеть пройти ревью по несколько раз (особенно, если это крупная фича, а ревью проходит очень медленно). Но я всегда подробно объясняю что делаю и зачем.
В наших силах проявлять инженерную культуру и уважение к коллегам, организуя ПР-ы таким образом, чтобы они были максимально атомарными и понятные для ревьюеров.
Читайте оригинальный пост и присоединяйтесь к обсуждению в Телеграм: @time2code