diff --git a/skills/requesting-code-review/CONTRIBUTORS b/skills/requesting-code-review/CONTRIBUTORS new file mode 100644 index 0000000..d8649da --- /dev/null +++ b/skills/requesting-code-review/CONTRIBUTORS @@ -0,0 +1 @@ +root diff --git a/skills/requesting-code-review/SKILL.md b/skills/requesting-code-review/SKILL.md new file mode 100644 index 0000000..bcd05b7 --- /dev/null +++ b/skills/requesting-code-review/SKILL.md @@ -0,0 +1,108 @@ +--- +name: requesting-code-review +description: 完成任务、实现重要功能或合并前使用,用于验证工作成果是否符合要求 +version: "1.0.0" +license: MIT +metadata: + hermes: + tags: [code-review] +--- + +# 请求代码审查 + +派遣代码审查子代理,在问题扩散之前发现它们。审查者获得的是精心组织的评估上下文——绝不是你的会话历史。这样可以让审查者专注于工作成果而非你的思考过程,同时保留你自己的上下文以便继续工作。 + +**核心原则:** 早审查,勤审查。 + +## 何时请求审查 + +**必须审查:** +- 子代理驱动开发中每个任务完成后 +- 完成重要功能后 +- 合并到 main 之前 + +**可选但有价值:** +- 卡住时(换个视角) +- 重构之前(建立基线) +- 修复复杂 bug 之后 + +## 如何请求 + +**1. 获取 git SHA:** +```bash +BASE_SHA=$(git rev-parse HEAD~1) # 或 origin/main +HEAD_SHA=$(git rev-parse HEAD) +``` + +**2. 派遣代码审查子代理:** + +使用 Task 工具,指定 `general-purpose` 类型,填写 `code-reviewer.md` 中的模板 + +**占位符说明:** +- `{DESCRIPTION}` - 你刚完成的内容简要说明 +- `{PLAN_OR_REQUIREMENTS}` - 预期功能 +- `{BASE_SHA}` - 起始提交 +- `{HEAD_SHA}` - 结束提交 + +**3. 处理反馈:** +- Critical 问题立即修复 +- Important 问题在继续之前修复 +- Minor 问题记录下来稍后处理 +- 如果审查者有误,用技术理由反驳 + +## 示例 + +``` +[刚完成任务 2:添加验证功能] + +你:让我在继续之前请求代码审查。 + +BASE_SHA=$(git log --oneline | grep "Task 1" | head -1 | awk '{print $1}') +HEAD_SHA=$(git rev-parse HEAD) + +[派遣代码审查子代理] + DESCRIPTION: 添加了 verifyIndex() 和 repairIndex(),支持 4 种问题类型 + PLAN_OR_REQUIREMENTS: docs/superpowers/plans/deployment-plan.md 中的任务 2 + BASE_SHA: a7981ec + HEAD_SHA: 3df7661 + +[子代理返回]: + 优点:架构清晰,测试真实 + 问题: + Important:缺少进度指示器 + Minor:报告间隔使用了魔法数字 (100) + 评估:可以继续 + +你:[修复进度指示器] +[继续任务 3] +``` + +## 与工作流的集成 + +**子代理驱动开发:** +- 每个任务完成后审查 +- 在问题叠加之前发现它们 +- 修复后再进入下一个任务 + +**执行计划:** +- 每个任务完成后或在自然 checkpoint 审查 +- 获取反馈,应用,继续 + +**临时开发:** +- 合并前审查 +- 卡住时审查 + +## 红线 + +**绝不要:** +- 因为"很简单"就跳过审查 +- 忽略 Critical 问题 +- 带着未修复的 Important 问题继续推进 +- 对合理的技术反馈进行争辩 + +**如果审查者有误:** +- 用技术理由反驳 +- 展示证明其可行的代码/测试 +- 要求澄清 + +参见模板:requesting-code-review/code-reviewer.md diff --git a/skills/requesting-code-review/code-reviewer.md b/skills/requesting-code-review/code-reviewer.md new file mode 100644 index 0000000..08bf9c4 --- /dev/null +++ b/skills/requesting-code-review/code-reviewer.md @@ -0,0 +1,166 @@ +# 代码审查员提示模板 + +派遣代码审查员子代理时使用此模板。 + +**用途:** 在工作成果扩散到更多工作之前,对照需求和代码质量标准做一次审查。 + +``` +Task tool(general-purpose): + description: "审查代码改动" + prompt: | + 你是一名资深代码审查员,精通软件架构、设计模式与最佳实践。 + 你的工作是对照计划或需求审查已完成的工作,在问题扩散之前发现它们。 + + ## 实现内容 + + {DESCRIPTION} + + ## 需求 / 计划 + + {PLAN_OR_REQUIREMENTS} + + ## 待审查的 Git 范围 + + **Base:** {BASE_SHA} + **Head:** {HEAD_SHA} + + ```bash + git diff --stat {BASE_SHA}..{HEAD_SHA} + git diff {BASE_SHA}..{HEAD_SHA} + ``` + + ## 检查内容 + + **计划对齐:** + - 实现是否匹配计划 / 需求? + - 偏差是有道理的改进,还是有问题的偏离? + - 计划中的所有功能都到位了吗? + + **代码质量:** + - 关注点分离清晰吗? + - 错误处理到位吗? + - 该有类型安全的地方有吗? + - DRY 但没有过早抽象? + - 边界情况处理了吗? + + **架构:** + - 设计决策合理吗? + - 可扩展性和性能合理吗? + - 有没有安全隐患? + - 与周围代码集成是否干净? + + **测试:** + - 测试验证的是真实行为,不是 mock? + - 边界情况覆盖了吗? + - 该有集成测试的地方有吗? + - 所有测试都通过吗? + + **生产就绪:** + - 如果改了 schema,有迁移策略吗? + - 考虑了向后兼容吗? + - 文档完整吗? + - 没有明显 bug? + + ## 校准标准 + + 按实际严重程度分类。不是所有问题都是 Critical。 + 在列出问题之前先认可做得好的地方——准确的肯定能让实现者 + 更愿意接受后续的反馈。 + + 如果发现与计划有重大偏差,明确标出,让实现者确认这个偏差 + 是不是有意为之。如果问题出在计划本身而不是实现,也要说清楚。 + + ## 输出格式 + + ### 优点 + [哪些地方做得好?具体一点。] + + ### 问题 + + #### Critical(必须修复) + [bug、安全问题、数据丢失风险、功能损坏] + + #### Important(应该修复) + [架构问题、缺失功能、错误处理不到位、测试漏洞] + + #### Minor(锦上添花) + [代码风格、优化机会、文档润色] + + 每个问题包含: + - File:line 引用 + - 哪里有问题 + - 为什么重要 + - 怎么修(如果不明显) + + ### 建议 + [关于代码质量、架构或流程的改进建议] + + ### 评估 + + **可以合并吗?** [是 | 否 | 修完再合] + + **理由:** [1-2 句技术评估] + + ## 关键规则 + + **要做:** + - 按实际严重程度分类 + - 具体(file:line,别含糊) + - 解释为什么这个问题重要 + - 认可优点 + - 给出明确判断 + + **不要:** + - 没检查就说"看起来 OK" + - 把小事标成 Critical + - 对没真看过的代码给反馈 + - 含糊其辞("改进错误处理") + - 回避给出明确判断 +``` + +**占位符说明:** +- `{DESCRIPTION}` —— 已构建内容的简要说明 +- `{PLAN_OR_REQUIREMENTS}` —— 预期功能(计划文件路径、任务文本或需求) +- `{BASE_SHA}` —— 起始 commit +- `{HEAD_SHA}` —— 结束 commit + +**审查员返回:** 优点、问题(Critical / Important / Minor)、建议、评估 + +## 输出示例 + +``` +### 优点 +- 数据库 schema 干净,迁移规范(db.ts:15-42) +- 测试覆盖全面(18 个测试,所有边界情况都覆盖) +- 错误处理有 fallback,做得很好(summarizer.ts:85-92) + +### 问题 + +#### Important +1. **CLI wrapper 缺少帮助文本** + - File: index-conversations:1-31 + - 问题:没有 --help flag,用户不会发现 --concurrency + - 修复:加 --help case 含使用示例 + +2. **缺少日期校验** + - File: search.ts:25-27 + - 问题:无效日期会静默返回空结果 + - 修复:校验 ISO 格式,抛错并附示例 + +#### Minor +1. **进度指示** + - File: indexer.ts:130 + - 问题:长操作没有 "X of Y" 计数 + - 影响:用户不知道要等多久 + +### 建议 +- 加进度上报改善用户体验 +- 考虑用配置文件管理排除项目(提升可移植性) + +### 评估 + +**可以合并吗:修完再合** + +**理由:** 核心实现扎实,架构和测试都很好。Important 问题(帮助文本、 +日期校验)很容易修,且不影响核心功能。 +```