Code review без вкусовщины

Когда использовать

Этот материал нужен, когда code review в команде регулярно превращается в спор о стиле, привычках и личных предпочтениях:

• один разработчик просит “сделать красивее”, но не объясняет риск;
• другой защищается фразой “так тоже работает”;
• review копится, потому что никто не понимает, что блокирует merge;
• часть замечаний полезна, часть похожа на вкусовщину;
• команда не отличает обязательное изменение от предложения.

Задача тимлида здесь не в том, чтобы лично решать каждый спор о коде. Его задача - помочь команде сделать правила review явными.

Главная проверка

Полезное замечание на review должно отвечать хотя бы на один вопрос:

• какой риск оно снижает;
• какую поддержку кода оно упрощает;
• какой сценарий или критерий приемки оно защищает;
• какую договоренность команды оно применяет;
• какое будущее изменение станет дешевле.

Если замечание нельзя связать ни с одним из этих вопросов, оно может быть личным предпочтением. Это не значит, что его нельзя обсуждать. Но его не стоит подавать как обязательное требование.

1. Разделить типы замечаний

Команде полезно договориться о четырех типах комментариев:

Такой словарь снижает напряжение. Комментарий “блокер” и комментарий “можно лучше назвать” перестают звучать одинаково.

2. Привязать review к готовности задачи

Code review не должен заново выяснять, что вообще должна была сделать задача.

Перед review должны быть понятны:

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

Если эти вещи неясны, проблема может быть не в коде. Возможно, задача была передана в разработку без достаточной готовности.

3. Назвать обязательные зоны проверки

Для большинства команд достаточно короткого набора:

• корректность сценария;
• ошибки и edge cases;
• данные и миграции;
• безопасность и доступы;
• совместимость;
• тесты;
• читаемость намерения;
• наблюдаемость и диагностика;
• влияние на поддержку.

Не каждое изменение затрагивает все зоны. Но если зона затронута, review должен обсуждать ее через риск, а не через “мне так больше нравится”.

4. Сделать стиль отдельной договоренностью

Стиль кода лучше автоматизировать или описать как явное правило.

Примеры:

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

Если правило повторяется на каждом review, его нужно вынести из личных комментариев в командную договоренность. Иначе команда будет снова спорить о том, что уже стало стандартом.

5. Не блокировать merge без риска

Блокировать merge стоит, если замечание влияет на:

• сценарий пользователя;
• сохранность или корректность данных;
• безопасность;
• совместимость;
• эксплуатацию;
• тестируемость;
• согласованную архитектурную границу;
• уже принятую командную договоренность.

Если замечание полезное, но не блокирующее, лучше назвать это явно:

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

Такая формулировка сохраняет качество разговора и не превращает каждый комментарий в приказ.

6. Где фиксировать спорное правило

Если спор повторяется, его не нужно решать заново в каждом pull request.

Варианты фиксации:

• командная договоренность;
• короткая запись решения;
• guideline рядом с кодом;
• автопроверка;
• шаблон PR;
• acceptance criteria для конкретного типа задачи.

Если решение влияет на архитектуру, публичный API, безопасность, тестовую стратегию или будущую поддержку, лучше сделать короткий decision record.

Шаблон договоренности по review

Зачем правило нужно:
Какой риск или повторяющийся спор оно закрывает.

Где применяется:
Все изменения / API / данные / UI / интеграции / критичные сценарии.

Что считается блокером:
Набор условий, при которых merge нельзя делать без исправления или явного решения.

Что считается предложением:
Полезные улучшения, которые не блокируют merge.

Как фиксируем исключение:
Комментарий в PR / задача / decision record / отдельный risk review.

Когда пересматриваем:
После нескольких PR, релиза, инцидента или изменения архитектуры.

Примеры формулировок

Слабое замечание:

Некрасиво, переделай.

Лучше:

Блокер: сейчас одно и то же состояние называется draft и pending.
В задаче есть критерий про отображение статуса, поэтому поддержка не сможет
понять, где заявка ожидает проверки, а где еще не отправлена.

Слабое замечание:

Мне не нравится этот if.

Лучше:

Не блокер. Предлагаю вынести проверку в отдельную функцию:
сейчас условие смешивает право доступа, статус заявки и feature flag,
поэтому следующий сценарий будет трудно добавить без ошибки.

Антипаттерны

Стоит остановиться, если:

• review проверяет вкус сильнее, чем риск;
• комментарии не отличаются по обязательности;
• ревьюер блокирует merge без объяснения цены;
• автор защищается вместо уточнения намерения;
• спор повторяется, но не становится правилом;
• команда ожидает качество от review, хотя задача была не готова;
• автоматизируемые вещи остаются предметом ручного спора.

Хорошее code review - это не место, где самый строгий человек побеждает. Это рабочая проверка, которая защищает результат, командные договоренности и будущую поддержку кода.