Code review и соглашения

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

Почему это важно. Ревью — то место, где знания расходятся по команде и где отлавливаются дорогие ошибки: архитектура, которую не получится расширить, дыра в безопасности, случай, о котором никто не подумал. И это же место, где команды теряют больше всего времени, споря о том, что должен был решить форматтер.

Что нужно понимать

• Зачем нужно это конкретное ревью: корректность, дизайн, обмен знаниями или всё сразу
• Какие комментарии блокирующие, а какие — предложения
• Что мог бы поймать инструмент, чтобы этим не занимался человек
• Что автор уже знает, а чего ему не хватает
• Когда ревью перестало приносить пользу и пора перейти к разговору

Ключевые темы

Как ревьюить

• Сначала описание, потом diff
• Корректность, крайние случаи и пути отказа — раньше, чем стиль
• Спрашивать, а не утверждать, когда вам может не хватать контекста
• Явно обозначать важность: блокирующее или «на ваше усмотрение»
• Апрувить, когда стало лучше, чем было, а не когда стало идеально

Как отдавать на ревью

• Небольшие изменения: внимание ревьюера не бесконечно
• Описание, которое объясняет зачем и какие варианты вы рассматривали
• Сначала прочитать собственный diff
• Отвечать по существу, а не защищаться

Что должны взять на себя инструменты

• Форматирование: решается форматтером и не обсуждается
• Линты для правил, которые команда повторяет из раза в раз
• CI как барьер для тестов и сборки, чтобы ревьюер их не проверял
• Автоматизация всего механического

Соглашения

• Именование, структура и раскладка файлов, записанные один раз
• Где живут соглашения и как они меняются
• Онбординг: новый человек должен иметь возможность просто прочитать правила
• Время от времени пересматривать сами соглашения

Как это ломается

• Ревью как узкое место: изменения лежат по нескольку дней
• Формальный апрув под давлением сроков
• Придирки, которые стоят дороже, чем экономят
• Один человек — единственный ревьюер целой области

Уровни

Уровень	Как это выглядит
Junior	Присылает небольшие изменения с описанием и конструктивно реагирует на замечания.
Middle	Ревьюит внимательно, замечает крайние случаи и проблемы дизайна, чётко отделяет блокирующее от необязательного.
Senior	Формирует культуру ревью, переносит механические проверки в инструменты и использует ревью, чтобы растить людей, а не только фильтровать код.

Практика

Для начала

• Прочитайте собственный diff Прежде чем звать ревьюера, прочитайте своё изменение чужими глазами и исправьте то, что найдёте.

• Разбейте большое изменение Возьмите pull request больше пятисот строк и разделите его на части, которые реально можно отревьюить.

• Помечайте важность На следующем ревью пометьте каждый комментарий как блокирующий или необязательный.

Глубже

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

• Измерьте ожидание Выясните, сколько изменения ждут ревью в вашей команде, и устраните главную причину.

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

Проверьте себя

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

Материалы

• Google's Code Review Developer Guide — самое полное публичное руководство: обе стороны процесса и критерий, по которому изменение принимают. Раздел о том, на что смотреть, применим напрямую.
• Conventional Comments — крошечное соглашение о том, как помечать важность комментария; снимает удивительно много трения.
• Effective Dart: Style — базовые соглашения для Dart, чтобы команда спорила о чём-нибудь другом.
• Dart linter rules — полный список. Стоит один раз прочитать целиком и найти правила, которые совпадают с повторяющимися комментариями на ревью в вашей команде.
• The Human Side of Code Review — о тоне и психологии, а именно от них зависит, помогает ревью команде или разъедает её.