Code review - практика чтения изменений перед влитием в основную ветку. На уровне инструмента - комментарии в Pull Request (см. pull-request). На уровне культуры - отдельный навык, влияющий на скорость команды больше, чем выбор фреймворка.
Зачем
Главные функции по убыванию:
- Распространение знания. Через год ревьюер всё ещё знает, что в этом куске кода происходит. Без ревью - только автор. Команда устойчивее к увольнениям и отпускам.
- Договорённость о стандарте. Каждое ревью - это разговор «как мы пишем тут». Стандарт держится живым.
- Поиск проблем. Логические ошибки, неучтённые edge case, проблемы производительности, безопасность. Автор замылен - ревьюер видит свежим взглядом.
- Защита от регрессий. Старший ревьюер ловит «эта правка сломает другую систему».
- Менторство. Обратная связь - главный способ роста новичка.
«Ловить баги» - пятое в списке. Большинство багов найдут тесты; ревью отлавливает то, что тесты не проверяют (архитектурные решения, читаемость, неявные допущения).
Хорошие практики автора
- Маленькие PR'ы. До 300 строк, в идеале - до 100. Ревьюер прочитает быстро, найдёт больше.
- Описание PR. Зачем, что, как проверить. Без описания ревьюер тратит первые 10 минут на восстановление контекста.
- Скриншоты для UI. Сэкономит ревьюеру локальный запуск.
- Self-review перед запросом ревью. Прочитай свой diff,
исправь очевидное - никто другой не должен находить за тебя
console.logили TODO.- Не обижайся на комментарии. Ревью - про код, не про автора. Если комментарий звучит обидно - обычно это привычка ревьюера, не злой умысел. Спроси «что ты предлагаешь?», не возражай в защиту.
Хорошие практики ревьюера
- Отвечай быстро. PR > 24 часов без ответа - авторе теряет контекст. Идеал - несколько часов.
- Объясняй, не приказывай. Не «исправь», а «здесь возможна проблема X - что думаешь?». Особенно с младшими.
- Различай уровни замечаний. Использовать префиксы:
nit:или(non-blocking)- мелочь, можно проигнорировать.suggestion:- идея, не обязательная к применению.must:или(blocking)- нужно поправить до merge.question:- просто вопрос, без претензии.- Замечание = причина. «Можно лучше» - плохо. «Здесь возможен
null reference, см. строку X в auth.py» - хорошо.
- Approve, когда готово. Не «approve с замечаниями» - это путаница. Если есть блокирующее - request changes. Если нет - approve.
Структура замечаний (стандарт Conventional Comments)
praise: Хорошо реализовано через композицию, чисто.
nit: Лучше переименовать `data` в `userResponse` - понятнее.
suggestion: Можно вынести в утилиту, использовать в trei местах.
question: Зачем тут try/catch без обработки?
issue (blocking): Этот код проглатывает ошибку - нужно либо
пробросить, либо логировать.
Префикс сразу говорит о тоне и обязательности. Сильно ускоряет обсуждение.
Что искать
Чек-лист «о чём думать при ревью»:
- Понятность. Через год другой разработчик прочитает и поймёт?
- Корректность. Решает ли задачу из описания PR?
- Edge cases. Что с null, пустым массивом, очень большими данными, гонкой?
- Тесты. Покрывают ли изменение? Тестируют ли поведение, а не реализацию?
- Безопасность. Пользовательский ввод санируется? Доступы проверяются?
- Производительность. Не появилось ли n² там, где должно быть n?
- Согласованность. Стиль соответствует остальному коду?
Не пытаться найти всё за один проход. Сначала understanding (понимаю ли я, что происходит), потом correctness, потом мелочи.
Подводные камни
- Bikeshedding. Бесконечные обсуждения цвета кнопки/имени переменной. Договорись с командой о time-box или линтер-правилах.
- Approve «не глядя». На больших PR это норма, но это значит, что ревью не работает. Решение - заставлять разбивать PR.
- Конфликт уровней. Senior ревьюит junior'а слишком жёстко, junior ревьюит senior'а слишком робко. Договоритесь, что в ревью нет иерархии - оба отвечают за качество.
- Эго-ревью. «Я бы написал по-другому». Если код решает задачу и читается - это легитимный вариант, не повод требовать переписать.