code-review について
リポジトリの規約と元のタスク要件に照らして変更を確認する、読み取り専用のコードレビュースキルです。比較基点の選定や指摘の検証方法を定め、2種類のレビュー結果を分けて報告します。
主な機能
- 01diffの比較基点の選定
- 02タスク要件と規約の確認
- 03規約とタスク範囲の個別レビュー
- 04変更内容に基づく指摘の検証
- 05指定されたファイルや分類の除外
- 06指摘の簡潔な個別報告
活用シーン
- 01ブランチの変更を基点と比較
- 02issueや依頼に対する変更の確認
- 03リポジトリ規約とセキュリティ上の懸念の確認
インストール
コマンドをコピーしてターミナルで実行します。
npx skills add https:/ / github.com/ ouyangjiahong26/ skills --skill code-reviewインストール後、Claude Code で/code-reviewを入力して読み込まれたか確認します。
詳細
プロジェクトの README より对 HEAD 与固定点之间的 diff 分别做标准审查和任务要求审查:
- 标准:代码是否遵守本仓库已记录的编码标准?
- 任务要求:代码是否忠实实现了原始 issue 的要求?
审查是只读的:不编辑文件、不提交、不跑构建或测试。发现的问题报出来,修复交给调用方。
流程
1. 确定审查目标
按这个顺序从参数认定固定点:
- PR/issue 引用(
#473、GitLab!67):按docs/agents/issue-tracker.md的流程拉取。PR 描述留给步骤 2 当任务要求,固定点取 PR 的 base 分支(origin/<base>)。 - 明确的 git 引用:commit SHA、分支名、tag、
main、HEAD~5、区间(main...my-feature)。 - 什么都没给:用
@{upstream}(上游跟踪分支)。 - 认不出,或没有上游跟踪:问用户。
一次把 diff 命令都记下来:git diff <fixed-point>...HEAD(三点,比较的是 merge-base),再加 git diff HEAD,覆盖已跟踪文件的未提交改动,再用 git status --short 找未跟踪文件(它们整体都是新改动,直接读全文)。未提交的改动同样在审查范围内,除非用户明确只审已提交内容。同时记下 commit 列表:git log <fixed-point>..HEAD --oneline。
继续之前,先跑一次上面的 diff 命令(坏引用会直接以 unknown revision 报错),确认固定点能解析、审查范围内有改动(提交区间、工作区、未跟踪文件至少一处非空)。坏引用、空范围、PR 场景的分支不匹配,按边界情况表处理,在这里就停下,不要拖到审查中途。
2. 找任务要求来源
按这个顺序找原始任务要求:
- 步骤 1 拉到的 PR 描述:参数里给了 PR 引用时,它就是首选任务要求。
- commit message 里的 issue 引用(
#123、Closes #45、GitLab!67等):按docs/agents/issue-tracker.md的流程拉取。 - 用户作为参数传入的路径。
- 本会话中用户的原始请求:这批提交出自当前会话时,用户当初的指令就是任务要求。
docs/、specs/、.scratch/下与分支名或功能匹配的任务要求文件。- 都没找到,问用户任务要求在哪。用户说没有任务要求,则跳过任务要求审查,报告无任务要求可用。
3. 找标准来源
标准来源是仓库里所有规定代码该怎么写的文件,比如 CODING_STANDARDS.md 或 CONTRIBUTING.md。
在仓库声明之上,标准审查始终再加两条基线:异味基线与安全基线。即使仓库什么都没声明,它们也适用。两条规则决定这两组基线怎么用:
- 仓库优先:已记录的仓库标准始终优先。仓库在某处明确认可了基线会标红的东西,就压住这条异味。
- 永远是判断题:每条异味是一个带标签的启发式判断,不是硬性违反。
每条异味先弄清是什么,再看怎么修,然后逐条对照 diff:
- 神秘命名:函数、变量或类型的名字没说清它做什么、装什么。修法:改名。想不出诚实的名字,是设计浑浊的信号。
- 重复代码:同样的逻辑形状出现在改动的多个 hunk 或文件里。修法:把共同的形状抽出来,各处都调用它。
- 依恋情结:一个方法更多地伸手到别的对象的数据里,而不是自己的。修法:把方法搬到它羡慕的那份数据上。
- 数据泥团:同样的几个字段或参数总是黏在一起出现(一个呼之欲出的类型)。修法:合并成一个类型,传这个类型。
- 基本类型偏执:用基本类型或字符串代替本该有自己类型的领域概念。修法:给概念一个自己的小类型。
- 重复分支:对同一类型的
switch/if阶梯在改动里反复出现。修法:用多态替换,或两边共享一张映射表。 - 霰弹手术:一个逻辑改动逼你在 diff 的许多文件里分散地改。修法:把一起变的东西收进一个模块。
- 发散式变化:一个文件或模块因几种互不相关的原因被改。修法:拆开,让每个模块只为一种原因变。
- 投机式泛化:为任务要求没提的需求加了抽象、参数或钩子。修法:删掉,内联回去,直到真实需求出现。
- 消息链:一长串
a.b().c().d()导航,调用方本不该依赖。修法:把这段导航藏进第一个对象的一个方法里。 - 中间人:一个类或函数主要只是转发。修法:删掉,直接调真正的目标。
- 被拒的遗赠:子类或实现者把继承来的大部分东西忽略或重写。修法:放弃继承,改用组合。
安全基线按下面的检查点逐项对照 diff:
- 注入:用户输入不经过参数化,直接拼进 SQL、命令或路径。
- 跨站脚本:未转义的用户输入进入 HTML 或 JS 上下文。
- 认证与授权:新增端点缺权限检查,或信任客户端传来的身份字段。
- 敏感信息:密钥、令牌、个人信息进入日志、错误信息或测试夹具。
- 危险求值:
eval、反序列化、动态加载作用于不可信输入。
4. 依次做标准审查和任务要求审查
先做标准审查,再做任务要求审查。每一项审查都先审后验,两步走完才算完成该项审查的报告。标准审查和任务要求审查的发现互不合并、互不重排。
分发:改动跨多个文件或模块时,按同一目录或同一模块分组,派子代理并行审,每组一个子代理。子代理的指令里直接贴该组的 diff(别让每个子代理重跑 git diff),并附上该组的审查任务说明和下面的丢弃标准。小改动自己审。子代理只负责读和报,不做修改。
审:按任务说明过一遍 diff,记下草稿发现。先划掉不审的部分(生成代码、*.lock、内嵌第三方代码、纯测试替身,以及仓库声明跳过或工具已经在管的类别:lint、格式、类型检查),在报告里记一行说明这些部分被排除,不对它们产生发现。
- 标准审查的任务:对照标准源文件(步骤 3)和上文的两条基线,按文件或 hunk 记录。(a) diff 违反了哪条已记录的标准,引用标准(文件 + 那条规则)。(b) 基线异味或安全检查点的命中,点名并引用对应 hunk。已记录标准的违反可以是硬性的,两条基线按步骤 3 的两条规则处理。
- 任务要求审查的任务:对照任务要求(路径或拉取到的内容)记录。(a) 任务要求提出、但在 diff 中缺失或只做了一半的需求。(b) diff 里有、但任务要求没提的行为(范围蔓延)。(c) 看似已实现、但实现方式看起来不对的需求。每条配任务要求的对应行。
diff 引入的类型、枚举成员、事件、消息、命令、队列项这类跨函数或模块接口传递的值,逐个读接收端的分发点(switch、路由、过滤器、处理器注册表、循环体),确认它由显式分支或已有的兜底分支正确接收。静默丢弃、无操作、return 了事都算发现。分发点常常在 diff 之外,读完接收端之前,不要断定生产端没问题。
验:对着 diff 逐条复核草稿发现:
- 把引用的事实核对回去:hunk 里真有那行,标准文件真有那条规则,任务要求真有那句要求。
- 站在被审代码的立场反驳一次,命中任一条就丢弃:影响不可证(说不出触发条件,也指不出受影响的调用方)。本次改动之前就存在。看着是作者有意的设计选择。需要本仓库别处都不具备的严格度才成立。只是改进式的模糊建议,没有离散的修法。
- 反驳不动、引用坐实的,保留原判,不降级成可能。
丢弃的发现不进报告,也不要提曾经发现又丢弃过。风格、命名、文档偏好本身不产生发现,仓库明文规定要审的除外。验证后的标准报告和任务要求报告各 400 字以内。
5. 汇总
把标准报告和任务要求报告分别放在 ## 标准 和 ## 任务要求 标题下,逐字或略加清理。不要合并或重排发现项,两个报告故意分开(原因见“为什么分开报告”)。
末尾一行总结:标准和任务要求各几条发现、各自最坏的问题(如有)是什么。不要在标准和任务要求之间选出单一的最坏,合并排序正是分开报告要避免的。
范围里有未提交或未跟踪改动时,在报告开头单列一行写明哪些文件不在提交区间内。
边界情况
| 情况 | 处理方式 |
|---|---|
固定点认不出,或既没有参数也没有 @{upstream} | 向用户询问固定点,拿到后从步骤 1 继续 |
| 审查范围内没有任何改动(提交区间、工作区、未跟踪文件都为空) | 停下,告诉用户无改动可审,不做标准和任务要求审查 |
| 参数是 PR 引用,但当前分支不是该 PR 的 head | 提示先检出 PR 分支(如 gh pr checkout <N>)再重跑。用户坚持就地审,改按该 PR 的 merge commit 对比并说明局限 |
| PR/issue 拉取失败 | 报出失败原因,问用户:贴任务要求原文,还是跳过 tracker 拉取、从步骤 2 第 3 条继续找 |
docs/agents/issue-tracker.md 不存在 | 让用户跑 /setup-ouyangjiahong-skills。用户不想配,跳过所有 tracker 拉取,从步骤 2 第 3 条继续找 |
| 仓库没有任何标准声明文件 | 标准审查照常进行,只用两条基线 |
| 改动全落在排除范围里(全是生成代码 / 锁文件 / 内嵌第三方代码) | 不做标准和任务要求审查,告诉用户改动的全部文件都被排除,并列出是哪些 |
| 子代理无法派出或超时 | 自己审全部。报告里写明哪些部分没覆盖到,不要默认声称覆盖完整 |
为什么分开报告
一个改动可以在一项审查里通过,在另一项里失败:
- 代码遵守每条标准,但实现的内容是错的:标准审查通过,任务要求审查失败。
- 代码恰好做了 issue 要求的事,但破坏了项目约定:任务要求审查通过,标准审查失败。
分开报告,防止一项审查的结论遮住另一项。