Skip to content

docs(review): strengthen PR review requirements#1161

Merged
ZR233 merged 1 commit into
devfrom
docs/strengthen-pr-review-requirements
Jun 6, 2026
Merged

docs(review): strengthen PR review requirements#1161
ZR233 merged 1 commit into
devfrom
docs/strengthen-pr-review-requirements

Conversation

@ZR233

@ZR233 ZR233 commented Jun 6, 2026

Copy link
Copy Markdown
Member

问题

当前 PR 审查规范已经存在于 .claude/skills/review-single-pr/SKILL.md,但项目记忆没有明确要求在审查 PR 前完整阅读该规范,也没有把基于规范逐项建立 todo/checklist 作为硬性流程。对于 bug 修复类 PR,现有表述也可以进一步明确为必须具备“修复后才能通过”的回归/复现测例。

修改

  • AGENTS.md 中增加 PR 审查约束:审查前必须完整阅读 .claude/skills/review-single-pr/SKILL.md,并基于完整规范建立 todo/checklist,逐项确认合并要求。
  • CLAUDE.md 中增加相同约束,确保 Claude 项目记忆与 AGENTS.mdreview-single-pr 技能保持一致。
  • review-single-pr 技能的 Normative Use 中明确要求:读取完整技能后、提交任何审查结论前,必须创建并逐项验证 merge-readiness checklist。
  • 强化 bug 修复要求:bug-fix PR 必须包含一个在未修复行为上失败、修复后才通过的 regression/reproduction test;除非有具体证据证明无法添加,否则视为阻塞问题。

方案逻辑

.claude/skills/review-single-pr/SKILL.md 继续作为唯一完整 PR 审查规范,避免新增文档副本导致规则漂移。项目记忆只放短约束,负责把未来审查流程导向该规范;具体执行细节仍集中在 skill 中维护。

验证

  • git diff HEAD~1..HEAD --check
  • rg -n "fully read|完整阅读|todo|checklist|regression|fails.*passes|修复" AGENTS.md CLAUDE.md .claude/skills/review-single-pr/SKILL.md

本次仅修改 Markdown 项目记忆和 skill 文档,不涉及 Rust 逻辑或 crate 变更,因此未运行 cargo fmt / cargo clippy

@ZR233
ZR233 marked this pull request as ready for review June 6, 2026 03:39
@ZR233
ZR233 merged commit 7ba0fbc into dev Jun 6, 2026
@ZR233
ZR233 deleted the docs/strengthen-pr-review-requirements branch June 6, 2026 03:40

@mai-team-app mai-team-app Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR 审查总结

变更内容

本 PR 在三份文档(AGENTS.mdCLAUDE.md.claude/skills/review-single-pr/SKILL.md)中增加 PR 审查流程约束:

  1. 技能文件 Normative Use 段:在读取完整技能后、提交审查结论前,强制创建 merge-readiness checklist,逐项覆盖 PR 元数据、CI、工作树、冲突处理、审查重点、重复分析、验证、阻塞发现、提交规则、审查分配和清理等要求。
  2. Bug 修复回归测试要求:将原有「要求复现测试」的简短描述强化为「要求在未修复行为上失败、仅在修复后才通过的回归/复现测试」,并要求审查者从测试代码、作者提供的红绿证据或本地红绿验证中确认。
  3. AGENTS.md / CLAUDE.md 项目记忆:各增加两条短约束,确保 Claude/Agent 项目记忆与技能文件保持一致。

实现逻辑

变更逻辑清晰:技能文件继续作为唯一完整审查规范,项目记忆仅放短约束,把审查流程导向该规范。这种分层设计避免了规则漂移。

验证

  • git diff HEAD~1..HEAD --check:通过,无空白问题
  • PR 声称的 grep 验证:在三份文件中均正确匹配到所有新增关键词(完整阅读todochecklistregression修复
  • CI:merge commit 上仅有 Release-plz PR 和 Release-plz release 两项检查,均成功通过(success=2, skipped=0, failure=0)
  • 本次仅修改 Markdown 文档,不涉及 Rust 代码,无需 cargo fmt / cargo clippy / cargo test

重复与重叠分析

  • base 分支上有大量 docs(review-single-pr) 相关历史提交(#1116#1113#1109#1082#1079 等),但本次 PR 聚焦于 checklist 要求和 bug 修复回归测试语言强化,与前序提交互补而非重复。
  • 当前 open PR(#1163 eBPF JIT 测试、#1160 Wayland app)与本 PR 改动完全无关,无重叠或冲突风险。

审查结论

无阻塞问题。PR 目标明确、改动精简、验证充分,已合入且 CI 通过。

建议(非阻塞)

CLAUDE.md 第 105 行写的是「not applicable with reason」,而 AGENTS.md 和技能文件写的是「not applicable with a concrete reason」,措辞略有不一致。建议后续 PR 统一为 with a concrete reason

Powered by mimo-v2.5-pro

Comment thread CLAUDE.md
- PR titles: Conventional Commits `type(scope): content`, e.g. `feat(axbuild): add board test flow`, `fix(starry-process): correct tty cleanup`
- PR titles in English, bodies in Chinese
- For any PR review, fully read (完整阅读) `.claude/skills/review-single-pr/SKILL.md` first; `AGENTS.md` and that skill are the review source of truth.
- Before deciding merge readiness, create a todo/checklist from the full `review-single-pr` requirements and verify each applicable item as satisfied, not applicable with reason, or blocking with evidence.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

「not applicable with reason」与 AGENTS.md 及技能文件中的「not applicable with a concrete reason」措辞不一致,建议补上 a concrete

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant