Ревью кода: что смотреть и чего не писать в комментариях

Ревью кода: что на самом деле проверяют и почему это не про баги

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

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

Четыре вещи, которые нужно проверить в первую очередь

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

1. Границы изменения

Что именно затрагивает это изменение? Если оно меняет публичный API, проверьте, что все зависимости обновлены и не сломаются после слияния. Если изменение затрагивает базу данных, убедитесь, что миграции обратимы: можно ли откатить их без потерь данных.

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

2. Имена

Имена переменных, функций, классов — это документация, которая всегда под рукой. Если имя не объясняет, что делает функция, разработчику придётся читать её реализацию. А это трата времени и источник ошибок.

Плохое имя — это не просто неудобство. Это сигнал о том, что автор не до конца понимает, что делает код. Если функция называется processData(), спросите, что именно она делает с данными. Если ответ не очевиден, имя нужно менять.

3. Обработка отказов

Что происходит, если API вернёт ошибку? Если база данных недоступна? Если пользователь введёт невалидные данные? Проверьте, что ошибки не игнорируются и не маскируются под успешное выполнение.

Обработка отказов — это не про “добавить try-catch”. Это про то, чтобы система оставалась в предсказуемом состоянии даже при сбоях. Если изменение не учитывает возможные ошибки, оно не готово к продакшену.

4. Обратимость

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

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

Что не должно попадать в комментарии

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

Форматирование

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

Вкусовые правки

“Я бы написал это иначе” — не аргумент. Если код работает и понятен, не трогайте его. Вкусовые предпочтения — это не повод для замечаний. Если автор написал if (condition) { return; }, а вам больше нравится unless (condition) { ... }, это не проблема кода.

Стилистические предпочтения

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

Очевидные ошибки

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

Как формулировать замечания, чтобы они не звучали как упрёк

Ревью — это обсуждение кода, а не оценка человека. Чтобы комментарий не звучал как упрёк, следуйте трём правилам:

1. Говорите о коде, а не о человеке. Не “ты забыл обработку ошибок”, а “здесь не хватает обработки ошибок”. Код — это отдельная сущность, и обсуждать нужно его, а не автора.
2. Объясняйте, почему это важно. Не “это плохо”, а “если здесь будет ошибка, пользователь увидит белый экран”. Объяснение помогает автору понять, почему замечание важно, и не повторять ошибку в будущем.
3. Предлагайте решение. Не “это не работает”, а “можно сделать так: …”. Предложение решения показывает, что вы не просто критикуете, а помогаете улучшить код.

Если замечание звучит как “ты неправ”, перепишите его. Цель ревью — сделать код лучше, а не доказать свою правоту.

Почему большие запросы получают поверхностное ревью

Чем больше изменений в запросе, тем меньше шансов, что его прочитают внимательно. Ревьюеру сложно удержать в голове контекст, если изменений больше сотни строк. А если запрос затрагивает несколько модулей, ревью превращается в угадывание: “а что здесь вообще происходит?”.

Что делать автору:

• Дробите изменения. Одно изменение — одна задача. Если нужно добавить функцию и исправить баг, сделайте два отдельных запроса. Это не только упрощает ревью, но и снижает риск конфликтов слияния.
• Пишите понятные описания. Если ревьюер не понимает, зачем это изменение, он не сможет его проверить. Описание должно объяснять, что делает изменение и почему оно важно.
• Не ждите идеального ревью. Если запрос большой, ревьюер пропустит что-то важное. Это нормально. Лучше получить быстрое ревью на небольшое изменение, чем ждать недели, пока кто-то разберётся в огромном запросе.

Если запрос на слияние больше 200 строк, считайте, что его не прочитают. А если больше 500 — что его вообще не откроют.

Цена задержки ревью: почему изменение, ждущее сутки, стоит дороже, чем кажется

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

1. Конфликты слияния. Чем дольше изменение ждёт ревью, тем больше шансов, что кто-то другой изменит тот же файл. А значит, автору придётся разрешать конфликты, тратить время на переработку и повторное тестирование.
2. Устаревание кода. Если изменение затрагивает активно развивающийся модуль, оно может устареть ещё до слияния. Автору придётся переделывать его под новые реалии, а это дополнительная работа.
3. Переключение контекста. Если ревью задерживается на неделю, автор вынужден переключаться на другие задачи. А потом возвращаться к старому контексту, вспоминать, что и зачем делал. Это трата времени и сил, которая не видна в трекере задач, но ощущается в конце месяца.

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

Тупик: требовать двух одобрений на каждое изменение

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

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

Позиция редакции

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

Это работает, если:

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

Но есть условие, при котором эта позиция неверна: если в команде нет доверия. Если разработчики не доверяют друг другу, ревью превращается в формальность. В этом случае нужно сначала решить проблему доверия, а потом думать о качестве ревью. Без доверия никакие правила не помогут.