Agent-almanac review-pull-request
install
source · Clone the upstream repo
git clone https://github.com/pjt222/agent-almanac
Claude Code · Install into ~/.claude/skills/
T=$(mktemp -d) && git clone --depth=1 https://github.com/pjt222/agent-almanac "$T" && mkdir -p ~/.claude/skills && cp -r "$T/i18n/zh-CN/skills/review-pull-request" ~/.claude/skills/pjt222-agent-almanac-review-pull-request-7a195f && rm -rf "$T"
manifest:
i18n/zh-CN/skills/review-pull-request/SKILL.mdsource content
审查拉取请求
端到端审查 GitHub 拉取请求——从理解变更到提交结构化反馈。所有 GitHub 交互均使用
gh CLI,并生成严重程度分级的审查评论。
适用场景
- 拉取请求已准备好审查并分配给你
- 在作者处理反馈后进行第二轮审查
- 在请求他人审查前审查自己的 PR(自我审查)
- 审计已合并 PR 的合并后质量评估
- 希望有结构化的审查流程而非随意扫描
输入
- 必填:PR 标识符(编号、URL 或
)owner/repo#number - 可选:审查重点(安全性、性能、正确性、风格)
- 可选:代码库熟悉程度(熟悉、较熟悉、不熟悉)
- 可选:审查时间预算(快速扫描、标准、彻底)
步骤
第 1 步:了解背景
阅读 PR 描述,理解此次变更要实现的目标。
- 获取 PR 元数据:
gh pr view <number> --json title,body,author,baseRefName,headRefName,labels,additions,deletions,changedFiles,reviewDecision - 阅读 PR 标题和描述:
- 此 PR 解决了什么问题?
- 作者采用了什么方法?
- 是否有作者希望重点审查的特定区域?
- 检查 PR 规模并评估所需时间:
PR 规模指南: +--------+-----------+---------+-------------------------------------+ | 规模 | 文件数 | 行数 | 审查方式 | +--------+-----------+---------+-------------------------------------+ | 小型 | 1-5 | <100 | 逐行阅读,快速审查 | | 中型 | 5-15 | 100-500 | 关注逻辑变更,略读配置 | | 大型 | 15-30 | 500- | 按提交审查,关注关键文件, | | | | 1000 | 标记是否应该拆分 | | 超大型 | 30+ | 1000+ | 标记需拆分。只审查最关键的文件。 | +--------+-----------+---------+-------------------------------------+
- 审查提交历史:
gh pr view <number> --json commits --jq '.commits[].messageHeadline'- 提交是否逻辑清晰且结构良好?
- 历史记录是否讲述了一个故事(每个提交是一个连贯的步骤)?
- 检查 CI/CD 状态:
gh pr checks <number>- 所有检查是否都通过了?
- 若检查失败,记录哪些失败了——这会影响审查方式
预期结果: 清楚了解 PR 的功能、存在原因、规模以及 CI 是否通过。此背景信息决定审查方式。
失败处理: 若 PR 描述为空或不清晰,将此作为第一条反馈。没有背景信息的 PR 是审查的反模式。若
gh 命令失败,验证你已通过认证(gh auth status)且有仓库访问权限。
第 2 步:分析差异
系统性地阅读实际代码变更。
- 获取完整差异:
gh pr diff <number> - 对于小型/中型 PR,顺序阅读完整差异
- 对于大型 PR,按提交审查:
gh pr diff <number> --patch # 完整补丁格式 - 对每个已更改的文件评估:
- 正确性:代码是否实现了 PR 所述的功能?
- 边缘情况:边界条件是否得到处理?
- 错误处理:错误是否被捕获并适当处理?
- 安全性:是否存在注入、认证或数据暴露风险?
- 性能:是否有明显的 O(n²) 循环、缺失索引或内存问题?
- 命名:新的变量/函数/类命名是否清晰?
- 测试:新行为是否有测试覆盖?
- 阅读时做记录,对每条观察进行严重程度分类
预期结果: 一组观察,涵盖差异中每个有意义变更的正确性、安全性、性能和质量。每条观察都有严重程度级别。
失败处理: 若差异太大无法有效审查,标记:"此 PR 更改了 {N} 个文件和 {M} 行。建议将其拆分为更小的 PR 以获得更有效的审查。"仍然审查风险最高的文件。
第 3 步:分类反馈
将观察整理为严重程度级别。
- 对每条观察进行分类:
反馈严重程度级别: +-----------+------+----------------------------------------------------+ | 级别 | 图标 | 描述 | +-----------+------+----------------------------------------------------+ | 阻塞 | [B] | 合并前必须修复。缺陷、安全问题、 | | | | 数据丢失风险、功能损坏。 | | 建议 | [S] | 应修复,但不阻塞合并。更好的方式、 | | | | 缺失的边缘情况、影响可维护性的风格问题。 | | 微调 | [N] | 可选改进。风格偏好、轻微命名建议、格式。 | | 称赞 | [P] | 值得指出的好工作。巧妙的解决方案、 | | | | 全面的测试、干净的抽象。 | +-----------+------+----------------------------------------------------+
- 对每个阻塞项,解释:
- 什么是错误的(具体问题)
- 为什么重要(影响)
- 如何修复(具体建议)
- 对每个建议项,解释替代方案及其更优之处
- 微调项保持简短——一句话足够
- 若有任何积极的方面,至少包含一条称赞
预期结果: 按严重程度排序的反馈项列表,级别清晰。阻塞项有修复建议。比例一般应为:少量阻塞、一些建议、极少微调、至少一条称赞。
失败处理: 若一切似乎都是阻塞的,PR 可能需要重新设计而非打补丁。考虑在 PR 级别请求变更而非逐行评论。若没有发现问题,直接说——"LGTM"(代码看起来不错)在代码良好时是有效的反馈。
第 4 步:撰写审查评论
撰写结构化、可操作的反馈。
- 撰写审查摘要(顶级评论):
- 一句话:PR 的功能(确认理解)
- 整体评估:批准、请求变更或评论
- 关键项目:列出阻塞问题(如有)和主要建议项
- 称赞:指出好的工作
- 为具体代码位置撰写内联评论:
# 通过 gh API 发布内联评论 gh api repos/{owner}/{repo}/pulls/{number}/comments \ -f body="[B] 此 SQL 查询存在注入漏洞。请改用参数化查询。\n\n\`\`\`suggestion\ndb.query('SELECT * FROM users WHERE id = $1', [userId])\n\`\`\`" \ -f commit_id="<sha>" \ -f path="src/users.js" \ -F line=42 \ -f side="RIGHT" - 一致地格式化反馈:
- 每条评论以严重程度标签开头:
、[B]
、[S]
或[N][P] - 对具体修复使用 GitHub 建议块
- 对风格/模式建议链接到文档
- 每条评论以严重程度标签开头:
- 提交审查:
# 批准 gh pr review <number> --approve --body "审查摘要" # 请求变更(存在阻塞问题时) gh pr review <number> --request-changes --body "审查摘要" # 仅评论(不确定或提供信息性反馈时) gh pr review <number> --comment --body "审查摘要"
预期结果: 已提交的审查包含清晰、可操作的反馈。作者确切知道需要修复什么(阻塞)、需要考虑什么(建议)以及做得好什么(称赞)。
失败处理: 若
gh pr review 失败,检查权限。你需要仓库的写入权限或是被指定的审查者。若内联评论失败,退而将所有反馈放在审查正文中,附文件:行引用。
第 5 步:跟进
跟踪审查解决情况。
- 在作者回应或推送更新后:
gh pr view <number> --json reviewDecision,reviews - 只重新审查处理了你反馈的变更:
gh pr diff <number> # 检查新提交 - 在批准前验证阻塞项已解决
- 随着问题被处理,解决评论线程
- 所有阻塞项修复后批准:
gh pr review <number> --approve --body "所有阻塞问题已解决。LGTM。"
预期结果: 阻塞问题已验证修复。审查对话已解决。PR 已批准或进一步请求变更,附具体剩余事项。
失败处理: 若作者对反馈有异议,在 PR 线程中讨论。关注影响(为什么重要)而非权威。若对非阻塞项的分歧持续,优雅地让步——作者拥有代码。
验证清单
- PR 背景已理解(目的、规模、CI 状态)
- 所有已更改文件已审查(或超大型 PR 的最高风险文件)
- 反馈已按严重程度分类(阻塞/建议/微调/称赞)
- 阻塞项有具体修复建议
- 积极方面已包含至少一条称赞
- 审查决定与反馈一致(只有在无阻塞项时才批准)
- 内联评论引用具体行,带严重程度标签
- CI/CD 检查已验证(批准前应为绿色)
- 作者修改后已完成跟进
常见问题
- 走过场批准:在没有实际阅读差异的情况下批准。每次批准都是对质量的断言
- 微调大轰炸:用风格偏好淹没作者。在时间紧迫的审查中省略微调;在辅导情境下再用
- 只见树木不见森林:在没有理解整体设计的情况下逐行审查。先阅读 PR 描述和提交历史
- 将风格问题设为阻塞:格式和命名几乎从不应该阻塞。将阻塞保留给缺陷、安全问题和数据完整性
- 无称赞:只指出问题令人沮丧。好的代码值得认可
- 审查范围蔓延:评论 PR 中未更改的代码。若预先存在的问题令你困扰,单独提一个 Issue
相关技能
— 系统级架构审查(对 PR 级审查的补充)review-software-architecture
— 对含有安全敏感变更的 PR 进行深度安全分析security-audit-codebase
— 流程的另一面:创建易于审查的 PRcreate-pull-request
— 清晰的提交历史使 PR 审查显著更容易commit-changes