Большинство чек-листов для ревью — это плацебо
Если у вашей команды есть чек-лист для code review, есть немалая вероятность, что он живёт на вики-странице, которую никто не открывает. В нём, скорее всего, написано что-то вроде „check for off-by-one errors” и „verify error handling”. Это правдивые утверждения. Но они слишком размыты, чтобы изменить поведение.
Чек-лист, который говорит вам „check for bugs”, — это не чек-лист. Это напоминание о том, что баги существуют.
Чек-лист в Fagan inspection служит другой цели. Это не список вещей, о которых нужно беспокоиться. Это структурированный инструмент, который направляет внимание inspector на конкретные категории дефектов, наблюдаемые в реальном codebase, который ревьюируется. Он строится на данных, адаптируется к типу артефакта и используется во время обязательной индивидуальной подготовки. При правильном применении это одна из главных причин, почему структурированная inspection находит в три-четыре раза больше дефектов, чем неформальное review.
Что делает чек-лист „структурированным”
Слово „структурированный” имеет здесь значение. Структурированный чек-лист — это не более длинный чек-лист. Это чек-лист с конкретными свойствами дизайна.
Во-первых, он выводится из данных об ускользнувших дефектах. Пункты берутся из багов, которые действительно попали в продакшен, а не из общего документа best practices. Если ваши последние три incidents включали race conditions в async cleanup, эта категория получает собственный пункт чек-листа. Если null dereferences не были проблемой в течение двух лет, этот пункт удаляется.
Во-вторых, он ограничен типом ревьюируемого артефакта. Fagan inspections изначально использовали разные чек-листы для документов требований, проектных документов, source code и test plans. Каждый артефакт имеет разные категории дефектов. Чек-лист проектного документа спрашивает о согласованности интерфейсов и coupling. Чек-лист source code спрашивает о boundary conditions и очистке ресурсов. Смешение их размывает оба.
В-третьих, он используется во время индивидуальной подготовки, а не во время встречи. Каждый inspector читает материал в одиночку, с чек-листом, до того как группа соберётся. Чек-лист формирует то, что каждый человек видит, читая код изолированно. Это не общий документ-справочник. Это персональная линза.
В-четвёртых, он достаточно короткий, чтобы быть применимым. Чек-лист с сорока пунктами — это каталог, а не инструмент. Fagan рекомендовал примерно от десяти до пятнадцати пунктов на тип артефакта. Это ограничение заставляет расставлять приоритеты. Вы оставляете категории, которые важны, и отбрасываете шум.
Как построить его из собственных данных
Лучший способ построить структурированный чек-лист — посмотреть, что уже пошло не так. Вот практичный процесс.
Начните с вашего incident tracker, bug database или заметок postmortem. Возьмите последние двадцать дефектов, которые ускользнули от review и попали в продакшен или тестирование. Категоризируйте каждый по root cause, а не по симптому. „Page crashed” — это симптом. „Missing null check after external API response” — это root cause.
Сгруппируйте root causes в категории. Вы, вероятно, обнаружите, что 80 процентов ваших ускользнувших дефектов попадают в три-пять категорий. Это ваши пункты чек-листа.
Для каждой категории напишите конкретный trigger-вопрос, а не размытое напоминание. „Check for nulls” размыто. „For every function that calls an external API, verify the response is validated before use” — это trigger-вопрос. Он точно говорит reviewer, что искать и где искать.
Вот конкретный пример. Допустим, ваша команда деплоит Python-сервисы, и вы проанализировали последние двадцать production issues. Вы находите такое распределение:
- 6 issues: отсутствие обработки ошибок на внешних вызовах
- 5 issues: неверные границы транзакций базы данных
- 4 issues: off-by-one в логике пагинации
- 3 issues: race conditions в кешированных данных
- 2 issues: логирование чувствительных данных
Ваш чек-лист должен содержать пять пунктов, по одному на каждую категорию. Каждый пункт должен быть trigger-вопросом, привязанным к вашим конкретным паттернам.
#!/usr/bin/env python3
"""Build a structured review checklist from escaped defect data."""
from collections import Counter
from dataclasses import dataclass
from typing import List
@dataclass
class EscapedDefect:
id: str
root_cause: str
trigger_question: str
def build_checklist(defects: List[EscapedDefect], max_items: int = 10) -> List[str]:
"""Build a Fagan-style checklist from escaped defect data.
Groups by root cause, sorts by frequency, and returns trigger
questions for the top categories.
"""
counts = Counter(d.root_cause for d in defects)
top_causes = counts.most_common(max_items)
# Map root causes back to their most representative trigger question
cause_to_question = {}
for d in defects:
if d.root_cause not in cause_to_question:
cause_to_question[d.root_cause] = d.trigger_question
checklist = []
for cause, count in top_causes:
question = cause_to_question[cause]
checklist.append(f"[{count}x] {question}")
return checklist
# Example: escaped defects from a Python web service
DEFECTS = [
EscapedDefect("BUG-101", "missing-external-error-handling",
"For every external API call, is the response validated before use?"),
EscapedDefect("BUG-102", "missing-external-error-handling",
"For every external API call, is the response validated before use?"),
EscapedDefect("BUG-103", "missing-external-error-handling",
"For every external API call, is the response validated before use?"),
EscapedDefect("BUG-104", "transaction-boundary-error",
"Does every database write have the correct transaction scope?"),
EscapedDefect("BUG-105", "transaction-boundary-error",
"Does every database write have the correct transaction scope?"),
EscapedDefect("BUG-106", "pagination-off-by-one",
"For every pagination query, are the limit and offset tested at boundaries?"),
EscapedDefect("BUG-107", "cache-race-condition",
"For every cached value, is there a clear invalidation or TTL strategy?"),
EscapedDefect("BUG-108", "sensitive-data-in-logs",
"Does any log statement include user data, tokens, or PII?"),
]
if __name__ == "__main__":
checklist = build_checklist(DEFECTS, max_items=5)
print("Structured Review Checklist")
print("=" * 40)
for item in checklist:
print(f" [ ] {item}")
Запустите этот скрипт на своих собственных баг-данных, и у вас будет чек-лист, привязанный к вашим реальным failure modes, а не к чужим.
Как на самом деле выглядит сессия review
С чек-листом в руках сессия review приобретает особый ритм.
Во время индивидуальной подготовки каждый inspector читает материал в одиночку со скоростью примерно 100–150 строк в час. Они используют чек-лист как линзу, а не как сценарий. Они не проходят по чек-листу пункт за пунктом по порядку. Они читают код естественно, и когда встречают паттерн, соответствующий категории чек-листа, останавливаются и тщательно изучают его.
Чек-лист не заменяет чтение. Это pattern matcher для вещей, которые ваш мозг, вероятно, пропустит.
На inspection meeting reader пересказывает код вслух. Когда inspector замечает потенциальный дефект, он тут же поднимает его. Категории чек-листа обеспечивают общий словарь. Вместо того чтобы сказать „это выглядит неправильно”, inspector может сказать „это похоже на ошибку границы транзакции, категория два”. moderator фиксирует это. author слушает. Никто не предлагает исправление.
Чек-лист не используется во время meeting. К этому моменту inspectors уже усвоили его. Встреча нужна для перекрёстной проверки того, что каждый нашёл в одиночку.
Почему универсальные чек-листы не работают
Большинство команд, которые пробуют чек-листы, бросают это дело, потому что используют неправильный тип.
Универсальный чек-лист, скопированный из блог-поста, не имеет эмоционального веса. Он просит reviewer искать вещи, которые, возможно, никогда не происходили в их codebase. Reviewer пробегает его взглядом, не видит ничего знакомого и возвращается к чтению diff, как делал всегда.
Структурированный чек-лист, построенный на ускользнувших дефектах, — другое дело. Каждый пункт представляет реальный incident, который стоил реального времени. Reviewer знает эти баги, потому что видел postmortems. Чек-лист связывает поведение review с конкретными, запоминающимися сбоями.
Другая распространённая ошибка — рассматривать чек-лист как инструмент для meeting, а не для подготовки. Если вы достаёте чек-лист во время review meeting, он превращается в сценарий для группового просмотра. Все читают один и тот же пункт, смотрят на один и тот же код и сходятся на одних и тех же очевидных наблюдениях. Сила чек-листа — в индивидуальной подготовке, где шесть человек независимо применяют одни и те же категории и находят разные вещи.
Бремя поддержки, которое большинство команд игнорирует
Чек-лист — не памятник. Это живой документ, который гниёт быстрее, чем код.
Если вы добавляете пункт в чек-лист каждый раз, когда что-то идёт не так, но никогда не удаляете, через полгода у вас будет сорок пунктов. На этом этапе reviewers начинают относиться к нему как к обоям.
Fagan рекомендовал пересматривать сам чек-лист после каждых нескольких inspections. Удаляйте пункты, которые не вызвали находку в последних пяти review. Добавляйте пункты только для новых категорий дефектов, которые ускользнули в продакшен. Держите общее число ниже пятнадцати. Если не можете удержать его коротким — вы не расставляете приоритеты.
Вот лёгкий скрипт для обрезки чек-листа на основе исторических данных о находках:
#!/usr/bin/env python3
"""Prune a review checklist: remove items that no longer trigger findings."""
from dataclasses import dataclass
from typing import List, Dict
@dataclass
class ChecklistItem:
category: str
trigger_question: str
findings_last_5_reviews: int
def prune_checklist(items: List[ChecklistItem], min_findings: int = 1) -> List[ChecklistItem]:
"""Remove checklist items that have not produced findings recently.
A Fagan-style checklist should be short enough to be usable.
Items that sit idle waste attention.
"""
kept = [item for item in items if item.findings_last_5_reviews >= min_findings]
removed = [item for item in items if item.findings_last_5_reviews < min_findings]
print(f"Kept {len(kept)} items, removed {len(removed)} items")
for item in removed:
print(f" REMOVED: [{item.category}] {item.trigger_question}")
return kept
# Example: current checklist with finding counts from the last 5 reviews
CURRENT_CHECKLIST = [
ChecklistItem("missing-external-error-handling",
"For every external API call, is the response validated before use?", 4),
ChecklistItem("transaction-boundary-error",
"Does every database write have the correct transaction scope?", 3),
ChecklistItem("pagination-off-by-one",
"For every pagination query, are the limit and offset tested at boundaries?", 0),
ChecklistItem("cache-race-condition",
"For every cached value, is there a clear invalidation or TTL strategy?", 1),
ChecklistItem("sensitive-data-in-logs",
"Does any log statement include user data, tokens, or PII?", 0),
]
if __name__ == "__main__":
pruned = prune_checklist(CURRENT_CHECKLIST, min_findings=1)
print("\nActive checklist:")
for item in pruned:
print(f" [ ] [{item.findings_last_5_reviews}x] {item.trigger_question}")
Планируйте этот review ежеквартально. Устаревший чек-лист хуже, чем его отсутствие, потому что он приучает reviewers игнорировать инструмент.
Компромисс: данные против скорости
Построение структурированного чек-листа из данных о дефектах требует времени. Нужна точная категоризация incidents. Нужна дисциплина, чтобы держать список коротким. Нужен процесс, чтобы поддерживать его актуальность.
Универсальный чек-лист пишется за пять минут и становится неактуальным за пять недель.
Структурированная версия медленнее создаётся, но быстрее используется. Reviewer с пятнадцатью целевыми trigger-вопросами сканирует код эффективнее, чем reviewer с сорока общими напоминаниями. Конкретика экономит время.
Настоящие издержки — организационные. Нужна запись об ускользнувших дефектах, достаточно детальная, чтобы извлечь root causes. У многих команд её нет. В их bug tracker заголовки вроде „user report: page broken” и нет последующего анализа. Без данных root cause невозможно построить чек-лист на основе доказательств. Можно только скопировать чужой.
Часто задаваемые вопросы
What is a structured checklist-based review?
Процесс review, в котором каждый inspector использует чек-лист, построенный на основе реальных данных об ускользнувших дефектах, во время обязательной индивидуальной подготовки перед групповым inspection meeting. Чек-лист содержит целевые trigger-вопросы, а не общие напоминания.
How is this different from a normal code review checklist?
Обычный чек-лист часто универсален, скопирован из шаблона и используется случайно во время review. Структурированный чек-лист выводится из конкретной истории дефектов вашей команды, ограничен типом артефакта и применяется во время сфокусированной индивидуальной подготовки с заданной скоростью чтения.
How many items should a structured checklist have?
От десяти до пятнадцати пунктов на тип артефакта. Больше — и reviewers начинают пролистывать. Меньше пяти — и вы, вероятно, упускаете категории.
How often should the checklist be updated?
После каждых трёх-пяти inspections или ежеквартально. Удаляйте пункты, которые не дали находок. Добавляйте пункты только для новых категорий ускользнувших дефектов.
Can this work without a full Fagan inspection process?
Да. Сам чек-лист — это самая переносимая часть метода Fagan. Требуйте индивидуальной подготовки, давайте reviewers структурированный чек-лист, построенный на ваших данных, и проводите time-boxed meeting с reader. Вы найдёте больше дефектов, чем при неформальном review, даже без полной six-phase ceremony.
Начните с одного модуля и одного квартала
Вам не нужно перестраивать весь процесс review. Выберите один модуль, в котором за последние шесть месяцев были production-дефекты. Соберите root causes. Постройте пятипунктный чек-лист. Потребуйте, чтобы два reviewer потратили тридцать минут на код и чек-лист до любой групповой дискуссии.
Зафиксируйте, что они находят. Сравните с тем, что обнаружил ваш обычный pull request review на том же коде. Это единственное сравнение скажет вам, стоит ли структурированный чек-лист затрат для вашей команды.
Если данные говорят да — расширяйтесь на ещё один модуль. Если данные говорят нет — ваш неформальный review уже достаточно хорош, или ваши данные о дефектах недостаточно детальны, чтобы построить полезный чек-лист. В любом случае у вас есть реальное измерение, а не чужое мнение.