Перейти к основному содержимому

Чеклист код-ревью: 11 правил, которые режут время ревью вдвое

· 8 мин. чтения
Artur Pan
CTO & Co-Founder at PanDev

Прямо сейчас у вашей команды несколько pull request'ов застряли в ревью. Скорее всего, три или больше. Один из них провисел пять дней. Исследование Google 2018 года (Sadowski et al., Modern Code Review: A Case Study at Google) показало: медианный review в Google закрывается за менее чем 4 часа. В большинстве команд, которые мы наблюдаем, этот показатель — 4 дня. Разница в 24 раза, и она почти полностью объясняется процессом, а не уровнем разработчиков.

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

Почему код-ревью ломается сегодня​

В командах с медленным циклом ревью повторяются три паттерна:

Паттерн 1: слишком большие PR. Исследование SmartBear State of Code Review показало: эффективность обнаружения дефектов резко падает, когда diff превышает ~400 строк. Выше 400, ревьюер скроллит глазами. Выше 1000, ставит «approve» не читая. В большинстве команд, которые мы измеряем, медиана размера PR — 350-600 строк, прямо у края обрыва.

Паттерн 2: восстановление контекста. Microsoft Research (Bacchelli & Bird, 2013, Expectations, Outcomes, and Challenges of Modern Code Review), ревьюер тратит до 30% времени ревью просто на попытку понять, что именно делает этот PR. Слабое описание — главный пожиратель времени в ревью.

Паттерн 3: ревью как налог. Ревьюер воспринимает ревью как interruption, а не как запланированную работу. Итог — штрафы за переключение контекста накапливаются: ревьюер открыл PR, прочитал 20 строк, получил пинг в Slack, закрыл вкладку, вернулся через три часа.

11 правил ниже бьют по всем трём паттернам одновременно.

Три фазы фреймворка код-ревью: Автор, Ревьюер, Команда. В каждой фазе свои правила, представлены в виде потокового диаграммы в пурпурно-розовом градиенте. Фреймворк одной картинкой: 3 правила автора, 4 правила ревьюера, 4 правила команды. Каждая фаза закрывает свой режим отказа.

Фаза 1: Дисциплина автора​

Автор контролирует 70% скорости ревью. Если он делает эти три вещи, у ревьюера появляется реальный шанс на быстрый оборот.

Правило 1: Лимит размера PR: 400 строк diff​

Жёсткий лимит. По данным SmartBear, обнаружение дефектов на час ревью пикует на 200-400 строках и быстро падает дальше. PR в 1200 строк не получит в 3 раза больше ревью, чем в 400 — он получит в 3 раза меньше. Разделяйте.

Размер PR (строк)Типичный итог
Меньше 100Ревью за минуты; 90%+ комментов по делу
100-400Оптимум; ревьюер дочитывает до конца
400-1000Ревьюер скроллит вторую половину; пропускает 40%+ проблем
Больше 1000Формальный approve; реального ревью нет

Исключение: генерированный код, конфиги, чистые тест-аддиции не считаются в 400. Считаются только файлы, где человек писал новую логику.

Правило 2: Структурированное описание PR​

Каждое описание PR отвечает на три вопроса, в таком порядке:

  1. Что изменилось? (один абзац)
  2. Зачем? (ссылка на тикет, мы используем нейминг веток feature/TASK-324, чтобы Jira сам подставил ссылку)
  3. Как проверить? (точные шаги, скриншоты или вывод тестов)

Если ревьюер не может за 30 секунд понять «что это вообще должно делать», описание сломано.

Правило 3: Самостоятельное ревью до запроса других​

Откройте свой PR. Прочитайте diff так, будто его писал незнакомец. Исследование Bacchelli & Bird показало: авторы, делающие self-review, ловят ~30% проблем, которые иначе ушли бы к ревьюеру. Стоимость — 5-10 минут, экономия — целый цикл ревью.

Фаза 2: Дисциплина ревьюера​

Ревьюер контролирует качество. Эти четыре правила предотвращают тихое снижение качества, которое подкрадывается, когда PR накапливаются.

Правило 4: Сессия ревью не больше 60 минут​

Когнитивная усталость — не метафора. По данным SmartBear, после 60 минут сфокусированного ревью уровень обнаружения дефектов падает более чем вдвое. Если PR требует больше часа — либо разделите его (см. Правило 1), либо запланируйте вторую сессию с перерывом.

Правило 5: Каждый комментарий — с меткой важности​

Только три уровня:

  • must-fix: блокирует merge
  • should-fix: стоит сделать, но не блокер
  • nit: вкусовщина, можно игнорировать

Без меток автор воспринимает каждый коммент как блокер. Команды, внедрившие эту конвенцию, сообщают об улучшении оборота ревью на 30-40% уже в первый месяц.

Правило 6: Никаких «LGTM» на нетривиальных изменениях​

«LGTM» («looks good to me») нормально для fix опечатки или bump зависимости. Для всего, где есть логика, требуйте от ревьюера сформулировать, что именно он проверил. Хватит одного предложения: «Проверил обработку таймаута в error-path; убедился, что миграция идемпотентна». Это предложение — артефакт ревью. И это же ловит фиктивные approve'ы при аудите.

Правило 7: Максимум 2 ревьюера: один эксперт предметной области + один пир​

Это контр-интуитивное правило и чаще всего нарушаемое. Добавление третьего ревьюера ощущается как «безопаснее» — но делает ревью медленнее И ниже качеством. Как только на PR назначены трое — включается bystander-эффект: каждый ревьюер подсознательно считает, что внимательно читает кто-то другой. Исследования групповых решений (классическая работа Latané & Darley плюс более свежие исследования инженерных команд) стабильно показывают: двойки дают более острую критику, чем тройки.

Правильная комбинация: один эксперт предметной области (корректность) + один пир (поддерживаемость и знание-sharing). Три ревьюера оправданы только для security-critical изменений, и даже там требуется явный sign-off от каждого.

Фаза 3: Дисциплина команды​

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

Правило 8: Первое ревью в течение 4 рабочих часов с открытия PR​

Не четыре часа календарного времени. Четыре рабочих часа — время, когда ревьюер реально онлайн и работает. Бенчмарк Google «медиана 4 часа» достижим для большинства команд, если это правило соблюдается. Мы отслеживаем это в 4 стадиях lead time — «время до первого ревью» это стадия 2, и именно здесь большинство команд теряет время.

Правило 9: Все автоматические проверки пройдены до ревью человеком​

Линтеры, тесты, security-сканы, build — всё это запускается при создании PR. Человек-ревьюер не должен тратить ни минуты на то, что поймает машина. Если ваш автоматический suite медленный (дольше 10 минут) — чините это до оптимизации ревью. Это upstream каждой метрики ревью.

Правило 10: Merge делает автор, не ревьюер​

После approve автор делает merge. Это сохраняет ownership: автор подтверждает, что согласен со всеми разрешёнными обсуждениями, проверяет, что ветка всё ещё зелёная против main, и отвечает за последствия. Merge от ревьюера создаёт «зомби-PR» — одобренные и заброшенные, потому что автор уже переключился на другую работу.

Правило 11: Эскалация после 48 часов без финального решения​

Если PR открыт 48 рабочих часов без merge или явного «не мержим». EM эскалирует. Кто-то заблокирован: либо ревьюер перегружен, либо автор избегает фидбека, либо приоритет команды неясен. 48-часовые PR, это сигнал о здоровье процесса, не об индивидуальной вине.

Частые ошибки, которые ломают систему​

ОшибкаПочему больноФикс
«LGTM» на PR в 900 строкРевьюер физически не мог это проверить; создаёт ложный audit trailПравило 1 + Правило 6
Автор не self-review'итРевьюер жжёт 10 мин на вещи, которые автор мог увидеть самПравило 3
3+ ревьюера на рутинуBystander-эффект; никто не читает внимательноПравило 7
Ревьюер пингует автора в чате вместо комментарияПереключает контекст обоимКоммент + @mention в PR
PR висит 3 дня, автор начинает новую работуАвтор теряет контекст; стоимость rework растётПравило 11
«Предложение» про whitespace как блокерЦиклы на субъективные предпочтенияПравило 5 (метка nit)

Чеклист, распечатайте и повесьте​

Автор:

  • PR меньше 400 строк человеческой логики
  • Описание отвечает Что / Зачем / Как проверить
  • Self-review сделан
  • Все авто-проверки зелёные

Ревьюер:

  • Сессия ≤ 60 минут (иначе, перерыв)
  • Каждый коммент помечен must-fix / should-fix / nit
  • Если одобряю, написал одно предложение про то, что проверил
  • Не больше 2 ревьюеров назначено

Команда:

  • Первое ревью началось в течение 4 рабочих часов
  • Merge делает автор после approve
  • Эскалируем любой PR, открытый > 48 рабочих часов

Как измерить, что это работает​

Отслеживайте эти инженерные метрики до и после внедрения. Ожидаемые сдвиги за 4-6 недель:

МетрикаСейчас типичноПосле цель
Медиана размера PR (строк)350-600≤ 400
Медиана времени до первого ревью1-3 дня< 4 рабочих часов
Медиана open-to-merge3-5 дней≤ 1 рабочий день
Rejected-after-approval rate8-15%< 5%
% комментов с меткой важности0%> 60%

PanDev Metrics отслеживает PR cycle time напрямую из Git-событий, привязанных к ID задач в трекере, эти числа видны по командам, репозиториям или конкретным ревьюерам в дашборде. Честное ограничение: мы меряем таймстемпы PR (создан, первое-ревью, merge), но не активные минуты, проведённые в UI ревью (скроллинг, комменты). Мы выводим вовлечённость ревьюера из пропусков его IDE-активности во время open-to-merge окна, это хороший прокси, но не прямое измерение.

Когда этот фреймворк не подходит​

Три сценария, где 11 правил нужно корректировать:

  1. Research или spike-ветки. Экспериментальный код, который не пойдёт в main, может игнорировать большую часть этого. Помечайте как WIP и пропускайте SLA на ревью.
  2. Соло-разработчики на early-stage проектах. Некому ревьюить. Выполняйте правила автора (особенно self-review), правила ревьюера отложите до роста команды.
  3. Массивные автоматические генераторы. Kubernetes-манифесты, генерированные API-клиенты, переводы, могут легитимно выдавать diff в 10 000 строк. Ревьюите генератор, не вывод.

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

Что читать дальше​

Готовы увидеть метрики своей команды?

30-минутная персональная демонстрация. Покажем как PanDev Metrics решает задачи именно вашей команды.

Забронировать демо