Как комментировать код на ревью: формула и примеры замечаний

Комментарий на code review полезен, если автор понимает три вещи: что именно заметил ревьюер, почему это важно и какой результат ожидается после правки. Формулировка «здесь неправильно» не отвечает ни на один из этих вопросов и почти гарантирует лишний круг обсуждения.

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

Формула хорошего комментария

Рабочая структура выглядит так:

Наблюдение → последствие → следующий шаг.

Например:

Здесь get() выбросит исключение, если запись уже удалена. Пользователь получит 500 вместо ожидаемого 404. Обработаем DoesNotExist на границе запроса?

В первой части указана конкретная строка и поведение. Во второй — эффект для системы или пользователя. В третьей — не приказ написать код определённым способом, а критерий исправления.

Не каждому комментарию нужны все три предложения. Если контекст очевиден, их можно сжать:

Этот запрос выполняется внутри цикла: для 100 элементов получится 101 обращение к базе. Здесь подойдёт предварительная загрузка связанных данных.

Главное — не убирать последствие. Именно оно отличает инженерное замечание от демонстрации знания правила.

Обсуждайте код, а не автора

Сравните две формулировки:

• «Ты забыл проверить права пользователя».
• «Здесь объект загружается только по id, поэтому пользователь может обратиться к чужой записи. Добавим ограничение по владельцу?»

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

Избегайте слов «очевидно», «просто» и «нормальный разработчик». Они не добавляют технической информации, но заставляют автора защищать себя вместо решения.

Называйте критичность явно

Если команда использует уровни blocker, major, minor и nit, укажите их в комментарии. Автору не придётся угадывать, препятствует ли замечание принятию PR.

Blocker

Используйте, когда изменение нельзя безопасно выпускать:

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

Major

Для существенной ошибки, которая влияет на корректность, надёжность или стоимость:

Major: повторный запуск задачи создаст второй платёж, потому что у операции нет идемпотентного ключа. Воркеры могут повторять задачу после таймаута; нужно защитить создание платежа от дублей.

Minor

Для локальной проблемы с ограниченным эффектом:

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

Nit

Для необязательного улучшения:

Nit: data здесь содержит набор активных заказов. Имя active_orders быстрее объяснит смысл, но это не блокирует PR.

Слово nit особенно полезно: оно разрешает автору не вносить правку, если выгода не оправдывает шум в diff.

Плохие и хорошие формулировки

Вместо «это небезопасно»

Слабый комментарий не объясняет модель угроз.

Лучше:

Значение попадает в SQL-строку без параметризации. Пользователь может изменить условие запроса через поле name. Передадим значение параметром драйвера?

Вместо «здесь будет медленно»

Слабый комментарий не показывает масштаб.

Лучше:

Для каждого заказа отдельно загружается пользователь. На странице из 50 заказов будет 51 запрос; стоит получить связь одним запросом.

Вместо «перепиши нормально»

Такая фраза не задаёт критерия готовности.

Лучше:

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

Вместо «почему не использовал паттерн?»

Название паттерна само по себе ничего не доказывает.

Лучше:

Новые типы уведомлений потребуют добавлять ещё одну ветку в этот if. Если список будет расширяться, таблица обработчиков уберёт изменение общей функции для каждого типа. Ожидаются новые варианты в ближайшее время?

Последний пример начинается с вопроса, потому что решение зависит от планов продукта. В code review необязательно заранее знать единственно правильную архитектуру.

Нужно ли предлагать готовое исправление

Для понятной локальной ошибки полезно дать направление. Это экономит время автора и показывает, что предложение реализуемо. Но писать полный патч в каждом комментарии не нужно: ревьюер отвечает за обнаружение риска и критерий исправления, а автор — за реализацию.

Чем критичнее замечание, тем важнее точность критерия:

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

Если вы не уверены в решении, так и напишите: «Вижу риск гонки между проверкой и записью, но не уверен, какая блокировка принята в этом сервисе. Как здесь обычно защищают операцию?» Точный вопрос полезнее уверенного, но неверного рецепта.

Как отвечать на несогласие автора

Возражение — нормальная часть ревью. Оно не означает, что автор игнорирует качество, а ревьюер обязан «победить».

Вернитесь к четырём опорам:

1. Как воспроизвести проблему?
2. Какой контракт нарушается?
3. Насколько вероятно событие?
4. Какова цена последствия?

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

Хорошая реплика в споре:

Согласен, повторная доставка сейчас редка, но брокер допускает её по контракту. Без уникального ключа последствия — двойное списание. Поэтому предлагаю оставить замечание блокирующим до теста на повторную обработку.

Плохая реплика:

Я всё равно считаю, что мой вариант лучше.

В первом случае решение связано с контрактом и ценой ошибки. Во втором обсуждается личный авторитет.

Как писать итоговый комментарий

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

Нашёл одну блокирующую проблему с проверкой доступа и два необязательных улучшения по структуре. После исправления доступа PR можно принимать; minor-комментарии можно вынести отдельно.

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

Как натренировать формулировки

Возьмите небольшой diff и ограничьте себя десятью минутами. Для каждого замечания проверьте:

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

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

В тренажёре code review с ИИ комментарии проверяются в диалоге: автор принимает точные замечания, просит конкретики у расплывчатых и возражает против спорных. В финале отдельно оцениваются ясность, тон, приоритизация и переговоры — именно те навыки, которые невозможно проверить выбором готового ответа.

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