Как проводить код-ревью по чек-листу с правильным тоном и типовыми замечаниями

Проводить код-ревью стоит как короткую управляемую проверку: сначала убедитесь в контексте и критериях приёма, затем просмотрите изменения по приоритетам (безопасность, корректность, поддерживаемость), и только потом обсуждайте стиль. Используйте чек-лист, фиксируйте тип замечания (баг/улучшение/стиль), пишите комментарии в нейтральном тоне и опирайтесь на автоматизацию там, где это возможно.

Краткий чек-лист для быстрого код-ревью

• Проверьте цель PR/MR и критерии приёма: что именно должно измениться и как это проверить.
• Сначала ищите баги и риски безопасности, затем - проблемы производительности, затем - читаемость и стиль.
• Каждое замечание маркируйте как: баг, улучшение или стилевое.
• Оставляйте комментарии с предложением действия: что поменять и почему, плюс быстрый способ проверить.
• Отдавайте форматирование, линт и базовые проверки CI: человеку - смысл, архитектуру, границы ответственности.

Подготовка: контекст и критерии приёма

Кому подходит: разработчикам уровня intermediate, тимлидам и владельцам компонентов, когда нужно стабильно повышать качество изменений и делиться знанием о кодовой базе. Если вы спрашиваете "как проводить код ревью" - начните именно с согласования контекста.

Когда не стоит делать: при отсутствии цели изменения (нет описания задачи/тикета), при "огромном" PR без разбиения, при недоступном окружении/логах, или когда обсуждение превращается в вкусовщину без правил проекта. В таких случаях эффективнее вернуть PR на доработку с просьбой добавить контекст или дробление.

• Минимум контекста: ссылка на задачу, ожидаемое поведение, как воспроизвести/проверить, ограничения (SLA, безопасность, совместимость).
• Критерии приёма: что считается "готово": тесты, миграции, обратная совместимость, документация, метрики/логи.

Структура ревью: приоритеты и последовательность

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

Что понадобится

• Доступ к репозиторию, CI-логам, артефактам сборки, окружению (dev/stage) или локальному запуску.
• Описание PR/MR: цель, границы изменения, ссылки на тикеты, план тестирования.
• Соглашения проекта: стиль кода, правила логирования, политика ошибок, требования безопасности.
• Базовый набор "инструменты для code review": дифф в платформе (GitHub/GitLab/Bitbucket), статический анализ/линтер, тесты, сканеры секретов (если приняты в команде).

Последовательность просмотра

1. Сначала смысл: прочитайте описание PR и убедитесь, что изменение соответствует задаче и не расширяет скоуп.
2. Потом границы: проверьте публичные API, контракты, миграции и обратную совместимость.
3. Затем критичное: безопасность, корректность, обработка ошибок, конкурентность, транзакции.
4. Далее качество: тесты, логирование, наблюдаемость, читаемость, повторяемость кода.
5. В конце стиль: форматирование и мелкие правки - только после того, как "большое" стало ясным.

Тон и формулировки: как давать конструктивную обратную связь

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

1. Классифицируйте замечание

   Явно определите, это баг (нужно исправить), улучшение (желательно), или стиль (по правилам проекта). Так вы снижаете напряжение и ускоряете решение.

   • Хорошо: "Баг: при null упадём на toString(); давай добавим guard и тест".
   • Плохо: "Тут всё плохо, перепиши нормально".
2. Опишите наблюдение, а не оценку

   Ссылайтесь на конкретный фрагмент диффа и ожидаемое поведение. Избегайте ярлыков ("криво", "некрасиво") - они не помогают исправлению.

   • Хорошо: "В этом месте исключение проглатывается, из-за чего клиент получит 200 без результата. Предлагаю вернуть ошибку/логировать и пробросить".
   • Плохо: "Странная обработка ошибок".
3. Предложите конкретное действие

   Дайте один рабочий вариант или 2-3 альтернативы, если есть trade-off. Укажите критерий готовности: что должно измениться, чтобы замечание считалось закрытым.

   • Хорошо: "Улучшение: вынести парсинг в функцию, добавить юнит-тест на некорректный формат; критерий - тест падает на старом коде и проходит на новом".
   • Плохо: "Нужно рефакторнуть".
4. Согласуйте уровень строгости

   Отделяйте "надо" от "можно". Если это стилевое замечание, и у команды нет правила - предложите вынести в отдельную задачу или договориться о правиле.

   • Хорошо: "Стиль: можем привести к соглашению по именованию? Если правила нет - ок, предлагаю обсудить и закрепить в линтере".
   • Плохо: "Мне не нравится название, переделай".
5. Закройте цикл проверки

   После правок перепроверьте именно те места, где был риск, и отметьте результат. Если автору нужно "обучение code review", предложите короткий разбор: что было критично и как избежать в следующий раз.

   • Хорошо: "Проверил: guard добавлен, тест есть, логирование не шумит. Спасибо - можно мёрджить".
   • Плохо: "Ну ладно, мёрдж" (без проверки, что замечания реально устранены).

Быстрый режим

1. Прочитайте описание и границы: цель, критерии приёма, что не трогали.
2. Пробегитесь по критичному: безопасность, ошибки, транзакции/конкурентность, ввод/вывод.
3. Сверьте тестирование: есть ли тесты и понятный способ проверить вручную.
4. Оставьте 3-5 самых важных комментариев: меньше, но точнее; каждое - с типом (баг/улучшение/стиль).

Типовые замечания: безопасность, производительность, читаемость

Используйте этот чек лист код ревью как проверку результата: после комментариев вы должны понимать, что риск снижен и поведение предсказуемо.

• Безопасность: нет ли утечек секретов, токенов, персональных данных в логах/исключениях.
• Валидация ввода: проверяются ли границы, формат, пустые значения, неявные преобразования.
• Авторизация/доступ: проверяется ли право на действие и принадлежность ресурса.
• Обработка ошибок: не глотаются ли исключения; корректны ли коды/типы ошибок; есть ли контекст в логах.
• Идемпотентность: повторный запрос не ломает состояние (если это важно для операции).
• Производительность: нет ли N+1, лишних аллокаций, тяжёлых операций в циклах, повторных запросов.
• Конкурентность: гонки, блокировки, транзакции, порядок операций, атомарность.
• Читаемость: ясные имена, небольшие функции, отсутствие "магических" констант, единая структура.
• Тестируемость: зависимости инъектируются, побочные эффекты изолированы, есть тесты на крайние случаи.

Инструменты и автоматизация: что оставить человеку, что - машине

Автоматизация ускоряет ревью, но не заменяет его: машина ловит регулярные ошибки, человек - смысл, границы и риски. Если вы выбираете инструменты для code review, держите правило: всё, что можно проверять автоматически и одинаково - отдавайте CI.

Что проверять	Лучше автоматизировать	Лучше проверять человеком	Почему так
Форматирование, базовый стиль	Да (formatter/линтер)	Только исключения	Снижает шум в обсуждениях и экономит время
Очевидные ошибки и анти-паттерны	Частично (статический анализ)	Да	Инструмент не знает контекст домена и допустимые компромиссы
Секреты в репозитории	Да (secret scanning)	Да (проверка логов/сообщений)	Нужны и сигнатуры, и здравый смысл
Покрытие тестами и запуск тестов	Да (CI)	Да (качество сценариев)	CI подтверждает факт запуска, человек оценивает ценность тестов
Архитектурные границы, контракты, UX/API	Редко	Да	Это про намерение и последствия, а не про синтаксис

Частые ошибки при автоматизации и ревью

• Пытаться "додавить" стиль вручную вместо включения форматтера в CI (получается бесконечный спор о вкусе).
• Принимать статический анализ как истину: предупреждение может быть нерелевантным, а реальная проблема - в другом месте.
• Смотреть только дифф, игнорируя контракты и соседний код (регрессии часто возникают на стыках).
• Мёрджить "красный" CI ради скорости (долг потом дороже и демотивирует команду).
• Сваливать всё в один комментарий без приоритета и типа (автор не понимает, что критично).
• Устраивать ревью как экзамен, а не совместную проверку (снижает качество и желание приносить изменения).
• Не фиксировать договорённости в правилах/линтерах (те же споры возвращаются снова).

Практика: примеры корректных и некорректных комментариев

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

Вариант 1: когда найден баг

• Корректно: "Баг: при пустом списке будет IndexOutOfBounds. Добавь проверку на пустоту и тест на этот кейс".
• Некорректно: "Опять упадёт, как обычно".

Вариант 2: когда нужно улучшение без блокировки

• Корректно: "Улучшение: эту логику можно вынести в отдельный метод, чтобы сократить когнитивную нагрузку. Не блокирую, но будет проще поддерживать".
• Некорректно: "Слишком длинно, переделай обязательно" (если это не влияет на корректность).

Вариант 3: когда это чисто стиль

• Корректно: "Стиль: давай выровняем именование по соглашению модуля (например, get*/fetch*). Если правила нет - предлагаю закрепить".
• Некорректно: "Переименуй, потому что мне так привычнее".

Вариант 4: когда нужен вопрос, а не утверждение

• Корректно: "Вопрос: что будет, если внешний сервис вернёт таймаут? Нужен ли ретрай/таймаут на клиенте и лог с корреляцией?"
• Некорректно: "Ты не подумал про таймауты".

Ответы на типичные вопросы о процессе ревью

Сколько комментариев оставлять за один проход?

Оставляйте столько, сколько реально улучшает изменение: сначала критичные (баг/безопасность), затем 1-3 самых полезных улучшения. Мелкий стиль лучше закрывать автоматизацией.

Что делать, если PR слишком большой?

Верните на разбиение и попросите план: по каким шагам автор разделит изменение и как вы будете принимать каждый. Большие диффы резко повышают шанс пропустить регрессию.

Как не скатиться в вкусовщину?

Маркируйте "стиль" отдельно и ссылайтесь на правило проекта или предложите закрепить его в линтере/гайде. Без правила - это предложение, а не требование.

Нужно ли запускать код локально во время ревью?

Для рискованных изменений - да: особенно когда затронуты миграции, конфиги, интеграции или сложная логика. Для простых правок обычно достаточно CI и понятного плана тестирования.

Как корректно спорить по архитектуре?

Фиксируйте критерий (простота, безопасность, расширяемость) и предложите 1-2 альтернативы с последствиями. Если решение влияет на модуль надолго - лучше вынести в короткое обсуждение и закрепить договорённость.

Как использовать ревью для роста команды?

Добавляйте краткий контекст к критичным замечаниям и ссылку на правило/практику; это и есть прикладное обучение code review. Раз в несколько недель полезно разбирать 1-2 PR как кейсы.

Когда можно мёрджить без полного ревью?

Только для согласованных исключений: срочный хотфикс с минимальным диффом и ясным риском, либо изменения без логики (например, правки текста) при действующем CI. Правило и границы должны быть заранее оговорены.