Code review: что действительно стоит искать, а что можно пропустить
Самый частый способ испортить code review — обсуждать на нём то, что может проверить машина. Пока в комментариях идёт спор о кавычках и длине строки, мимо проходит незакрытое соединение и запрос в цикле.
Полезное ревью начинается с признания простого факта: внимание рецензента — ограниченный ресурс. Его нужно тратить там, где машина бессильна.
Иерархия замечаний
Замечания стоит упорядочить по цене пропуска — от самых дорогих к самым дешёвым.
1. Корректность и безопасность
То, из-за чего систему поднимают ночью: потеря данных, гонки, дыры в авторизации, необработанные ошибки, неверная обработка пограничных значений. Здесь рецензент незаменим, потому что нужно понимать намерение, а не только текст.
Классический пример — проверка прав, которая выглядит правильной, но выполняется после действия:
def delete_comment(request, comment_id):
comment = Comment.objects.get(pk=comment_id)
comment.delete() # удалили
if comment.author != request.user: # и только потом проверили
raise PermissionDenied
Линтер к этому равнодушен: синтаксис безупречен. Заметить может только человек, который читает код как последовательность событий.
2. Устойчивость к изменениям
Не «красиво ли написано», а «что придётся переписать, когда требования изменятся». Здесь живут вопросы про границы модулей, скрытую связность и неявные зависимости от порядка вызовов.
Хороший вопрос на ревью: «если завтра источник данных станет другим, сколько файлов придётся тронуть?» Ответ часто вскрывает проблему быстрее любого спора об абстракциях.
3. Читаемость для следующего
Имена, которые врут, комментарии, объясняющие «что» вместо «почему», функция на двести строк с четырьмя уровнями вложенности. Это дешевле ошибки, но дороже форматирования: следующий человек в этом файле потратит часы.
4. Стиль
Отступы, порядок импортов, кавычки, максимальная длина строки. Всё это должно быть автоматизировано и не появляться в комментариях вовсе. Если инструмент не настроен — правильное замечание на ревью звучит как «давайте добавим правило в конфиг», а не «поправь здесь».
Размер диффа решает больше, чем усердие
Качество ревью падает нелинейно с объёмом. На сотне строк рецензент разбирается в логике; на тысяче — просматривает по диагонали и ставит одобрение, потому что честно проверить такой объём стоит полдня.
Практическое следствие: если вы хотите настоящего ревью, дробите изменения. Рефакторинг — отдельно от функциональности. Переименование — отдельным коммитом. Смешивать перенос файлов с изменением логики — верный способ получить формальное одобрение.
Как формулировать замечания
Комментарий полезен, когда его можно выполнить, не угадывая намерение автора. Помогают три вещи: указать проблему, объяснить последствие, предложить направление.
Сравните: «здесь плохо» — против «этот запрос выполняется в цикле, на
тысяче записей получится тысяча обращений к базе; можно вынести
в один запрос с select_related».
Отдельно стоит различать блокирующие замечания и пожелания. Явная пометка «необязательно» экономит переписку и снимает ложное ощущение, что автор обязан согласиться со всем подряд.
Что автоматизировать до ревью
Всё, что имеет однозначно правильный ответ, должно проверяться в CI и не доходить до человека:
- форматирование и стиль — форматтер и линтер;
- типовые ошибки — статический анализ и проверка типов;
- известные уязвимости в зависимостях — аудит пакетов;
- поведение — тесты, желательно с измерением покрытия именно изменённых строк.
Чем больше уровней проходит код до ревью, тем содержательнее разговор на самом ревью.
Что в итоге
Ревью — это не проверка на соответствие стилю, а последний рубеж перед продакшном, где ещё есть человек с контекстом. Тратьте это внимание на то, что действительно ломается: корректность, безопасность и способность кода пережить следующее изменение. Остальное поручите инструментам.