zemlan.in

Мысли про Pull Request Assignee

Внимательно подбирай аудиторию

В завершении записи про создание pull request’а, я вскользь упомянул assignee

Назначь в assignee заинтересованных коллег (будь то отдельные юзернеймы, целые команды, или только ответственные за ревью)

Будто бы это настолько легко и настолько малозначительно…


Серия записей про code review:

  1. Зачем?
  2. Создание Pull Request’а
  3. Мысли про Pull Request Assignee
  4. Code Review & Response

You are Reviewer Zero

Нулевым ревьювером является сам автор pull request’а. Конечно, ничто не мешает забить и переключиться на другую задачу после того, как передал эстафету коллегам, но просмотр уже знакомого кода в другом окружении (UI code review вместо UI code editor’а) или спустя некоторое время (когда все «очевидности» забылись) поможет самостоятельно заметить опечатку или какой-нибудь странный момент

Я иногда делаю ещё так - после публикации своего PR я сам же ему делаю Review (в Github можно прокомментировать свой PR, но без Approve). Это позволяет добавить комментарии/контекст конкретно к тому коду, которого это касается

— хороший комментарий к прошлой записи

Особенно удобно это сделать в черновом pull request’е, который вроде как уже создан и запустил свои проверки на CI, но ещё не сообщил о своём существовании code owner’ам

Кому смотреть

В небольших проектах с плотной командой, вопрос «кому кинуть на ревью» решается сам собой — ты либо кидаешь всем (потому что вас немного), либо ты знаешь ответственную за ту или иную часть кода и кидаешь ей

В более… динамических проектах и командах, из-за размеров и слабой связи, ситуация может быть менее простой. Зачастую, это решается указанием в уже упомянутых code owner’ах либо «владельцев мнений» (тимлиды/техлиды/сеньоры/помидоры), либо целых команд. Это влечёт за собой одну из двух проблем: «много нас, а он один» и «много вас, а я один»

Много нас, а он один

Команда в ревьюверах порождает риск того, что каждый участник команды посчитает, что просмотр твоего pull request’а — чужая задача. Наличие «дежурных»/«oncall» в команде не спасёт — если pull request был создан вечером пятницы, когда я был дежурным, то в понедельник я буду думать «теперь это проблема сегодняшней дежурной Лены» и забить. Лена, в свою очередь, тоже может справедливо забить — pull request же пятничный, а в пятницу дежурила не она

Плюс, новички в команде могут стесняться комментировать — вдруг что-то не заметят или зададут «глупый» вопрос. А если в ревьюверах вся остальная команда, то можно затаиться и переждать

Команда в assignee может иметь и противоположный эффект, когда все-все-все спешат оставить комментарий, не особо вдаваясь в подробности1. В теории, автор pull request’а может вложить время и силы на ответ всем "зевакам", но это сложно, изнуряет, и может требовать оправданий начальству, если оно считает что «твоя задача — решать проблемы с кодом, а не учить джунов в других командах»

Много вас, а я один

Наличие кого-то, через кого будут проходить все pull request’ы, конечно, имеет свои плюсы, например, более консистентный стиль и высокий шанс заметить конфликт с неявным допущением в проекте

Но глупо ожидать (или требовать), что один человек будет внимательно пропускать через себя весь контекст с той же скоростью, с которой пишет код вся остальная команда вместе взятая. В треугольнике «много/быстро/внимательно» все три выбрать нельзя. Даже два не всегда получается…

Я не знаю, как однозначно решить обе проблемы. Но уверен, что:


После этого отступления, теперь точно можно говорить о процессе с точки зрения ревьюверов


  1. Спасибо Кате за напоминание ↩︎