大多数审查检查清单都是安慰剂

如果你的团队有一份代码审查检查清单,它很有可能躺在没人打开的维基页面上。上面大概写着”check for off-by-one errors”和”verify error handling”之类的话。这些话都是对的。但也过于笼统,不足以改变行为。

一份告诉你”check for bugs”的检查清单不是检查清单。它只是提醒你bug存在而已。

Fagan inspection中的检查清单有不同的用途。它不是一份担忧清单,而是一种结构化工具,将审查者的注意力引向在实际被审代码库中观察到的特定缺陷类别。它由数据构建,针对工件类型量身定制,并在强制性个人准备阶段使用。如果使用得当,它是结构化检查能发现非正式审查三到四倍缺陷的主要原因之一。

什么让检查清单”结构化”

“结构化”这个词在这里很重要。结构化检查清单不是更长的检查清单,而是具有特定设计属性的检查清单。

首先,它源自逃逸缺陷数据。条目来自实际进入生产环境的bug,而不是来自通用最佳实践文档。如果你最近三次事故都涉及异步清理中的race conditions,那么该类别就会有自己的检查清单条目。如果null dereferences两年来都不是问题,那么该条目就会被移除。

其次,它仅限于被审查的工件类型。Fagan inspection最初对需求文档、设计文档、源代码和测试计划使用不同的检查清单。每个工件都有不同的缺陷类别。设计文档检查清单询问接口一致性和耦合度。源代码检查清单询问边界条件和资源清理。将两者混在一起会稀释两者。

第三,它在个人准备期间使用,而不是在会议期间。每位检查员在小组集会之前,独自带着检查清单阅读材料。检查清单塑造了每个人在孤立阅读代码时看到的内容。它不是共享参考文档,而是个人透镜。

第四,它足够短,便于使用。四十个条目的检查清单是目录,不是工具。Fagan建议每个工件类型大约十到十五个条目。这种限制迫使进行优先级排序。保留重要的类别,丢弃噪音。

如何从自己的数据中构建

构建结构化检查清单的最佳方法是查看已经出了什么问题。以下是一个实用的流程。

从你的事故跟踪器、bug数据库或事后分析记录开始。提取最近二十个逃脱审查并进入生产或测试阶段的缺陷。按根本原因而非症状对每个缺陷进行分类。“Page crashed”是症状。“Missing null check after external API response”是根本原因。

将根本原因分组为类别。你可能会发现80%的逃逸缺陷落入三到五个类别。这些就是你的检查清单条目。

对每个类别,写出一个具体的触发问题,而不是模糊的提醒。“Check for nulls”是模糊的。“For every function that calls an external API, verify the response is validated before use”是一个触发问题。它告诉审查者确切要找什么以及在哪里找。

以下是一个具体例子。假设你的团队发布Python服务,你分析了最近二十个生产问题。你发现如下分布:

  • 6个问题:外部调用缺少错误处理
  • 5个问题:数据库事务边界错误
  • 4个问题:分页逻辑中的off-by-one
  • 3个问题:缓存数据中的race conditions
  • 2个问题:记录敏感数据

你的检查清单应该有五个条目,每个类别一个。每个条目应该是一个与你的特定模式绑定的触发问题。

#!/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}")

针对你自己的bug数据运行此脚本,你将拥有一份绑定到你实际失效模式而非他人失效模式的检查清单。

审查会议实际是什么样子

拿着检查清单,审查会议有特定的节奏。

在个人准备期间,每位检查员以大约每小时100到150行的速度独自阅读材料。他们将检查清单用作透镜,而不是脚本。他们不会按顺序逐项过检查清单。他们自然地阅读代码,当遇到与检查清单类别匹配的模式时,会停下来仔细审查。

检查清单不能替代阅读。它是大脑可能会跳过的事物的模式匹配器。

在检查会议中,阅读者大声转述代码。当检查员发现潜在缺陷时,会立即提出。检查清单类别提供共享词汇。检查员不必说”这看起来不对”,而是可以说”这看起来像是事务边界错误,类别二”。主持人记录。作者倾听。没有人提出修复方案。

会议期间不参考检查清单。到那时,检查员已经将其内化。会议是为了交叉验证每个人独立发现的内容。

为什么通用检查清单会失败

大多数尝试使用检查清单的团队会放弃,因为他们使用了错误的类型。

从博客文章中复制来的通用检查清单没有情感分量。它要求审查者寻找可能在其代码库中从未发生过的事情。审查者浏览一遍,看不到熟悉的内容,然后回到他们一贯阅读diff的方式。

由逃逸缺陷构建的结构化检查清单则不同。每个条目代表一个耗费了真实时间的真实事故。审查者知道这些bug,因为他们看过事后分析。检查清单将审查行为与具体、难忘的错误联系起来。

另一个常见错误是将检查清单视为会议工具而非准备工具。如果你在审查会议中拿出检查清单,它就变成了集体浏览的脚本。所有人读同一个条目,看同一段代码,收敛到同样的明显观察。检查清单的力量在于个人准备,六个人独立应用相同类别并发现不同问题。

大多数团队忽视的维护负担

检查清单不是纪念碑。它是比代码腐烂得更快的活文档。

如果你每次出问题都增加一个检查清单条目但从不删除,六个月后你会有四十个条目。到那时,审查者开始将其当作壁纸对待。

Fagan建议每隔几次检查后就审查检查清单本身。移除最近五次审查中未触发发现的条目。仅对逃逸到生产环境的新缺陷类别增加条目。将总数保持在十五个以下。如果你无法保持简短,说明你没有在做优先级排序。

以下是一个基于历史发现数据修剪检查清单的轻量脚本:

#!/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}")

每季度安排一次这种审查。过时的检查清单比没有检查清单更糟,因为它训练审查者忽视这个工具。

权衡:数据与速度

从缺陷数据中构建结构化检查清单需要时间。你需要准确的事故分类。你需要保持清单简短的纪律。你需要一个保持其最新的流程。

通用检查清单五分钟写完,五周后变得无关紧要。

结构化版本创建更慢但使用更快。有十五个针对性触发问题的审查者比有四十个通用提醒的审查者更高效地扫描代码。具体性节省时间。

真正的成本是组织性的。你需要足够详细的逃逸缺陷记录来提取根本原因。许多团队没有。他们的bug跟踪器标题像”user report: page broken”,没有后续分析。没有根本原因数据,你无法从证据构建检查清单。你只能复制别人的。

常见问题

What is a structured checklist-based review?

一种审查流程,每位检查员在小组检查会议前的强制性个人准备期间,使用基于实际逃逸缺陷数据构建的检查清单。检查清单包含针对性触发问题而非通用提醒。

How is this different from a normal code review checklist?

普通检查清单通常是通用的,从模板复制,在审查期间随意参考。结构化检查清单源自团队特定的缺陷历史,限于工件类型,并在固定阅读速度下的集中个人准备期间使用。

How many items should a structured checklist have?

每个工件类型十到十五个条目。超过这个数量,审查者开始浏览。少于五个,你可能遗漏了类别。

How often should the checklist be updated?

每三到五次检查后,或每季度一次。移除未产生发现的条目。仅对新的逃逸缺陷类别增加条目。

Can this work without a full Fagan inspection process?

可以。检查清单本身是Fagan方法中最可移植的部分。要求个人准备,给审查者一份基于你们数据构建的结构化检查清单,并与阅读者一起进行限时会议。即使没有完整的六阶段仪式,你也会发现比非正式审查更多的缺陷。

从一个模块和一个季度开始

你不需要彻底改革整个审查流程。选择一个在过去六个月中有过生产缺陷的模块。收集根本原因。构建一个五条目检查清单。要求两位审查者在任何小组讨论之前花三十分钟阅读代码和检查清单。

记录他们发现的内容。与同一代码上常规拉取请求审查发现的内容进行比较。这唯一的一次比较将告诉你结构化检查清单是否值得你的团队付出努力。

如果数据说是,扩展到一个更多模块。如果数据说否,你的非正式审查已经足够好,或者你的缺陷数据不够详细,无法构建有用的检查清单。无论哪种情况,你得到的是真实测量而非借来的观点。