pr-review

PR 质量审查

全面审查拉取请求或功能分支的质量。此技能涵盖生产就绪性、代码质量、用户体验、文档、测试和安全性。

审查按七个维度组织成一份检查清单。针对每个维度，调查实际代码并报告具体发现——不要只说“看起来不错”，而要提供文件路径和行号。

开始

首先，了解更改的范围：

# What branch are we on, what's the base?
git branch --show-current
git log main..HEAD --oneline

# What files changed?
git diff main...HEAD --stat

# Full diff for review
git diff main...HEAD

如果审查的是远程 PR，请先获取它：

gh pr view <number> --json title,body,files
gh pr diff <number>

深入检查代码前，先阅读 PR 描述和所有提交，以了解其意图。

审查维度

逐项检查以下每个维度。针对每项报告状态：

• 通过——符合要求，没有问题
• 需要改进——发现了具体问题（请列出）
• 不适用——与此更改无关

1. 生产就绪性

此代码的行为是否正确，并能妥善处理故障？

• 错误处理：是否检查了所有错误？是否使用上下文包装错误（fmt.Errorf("context: %w", err)）？是否向用户提供了可采取行动的错误消息？
• 边界情况：空输入、nil 值、缺失配置、网络故障、API 速率限制、大型数据集、分页边界
• 安全检查：所有更改状态的命令（create、edit、apply、delete、update）都必须在 LoadConfig() 之后、客户端操作之前进行安全检查。模式如下：

  checker, err := NewSafetyChecker(cfg)
  if err := checker.CheckError(safety.OperationXXX, safety.OwnershipUnknown); err != nil { return err }
  

  验证操作类型是否正确。仅在 dry-run 路径中跳过。
• 库代码不得向 stdout 输出内容：pkg/ 必须返回数据，而不是打印。只有 cmd/ 负责处理输出。
• 不得硬编码机密或客户数据：代码或测试中不得包含真实姓名、环境 ID、令牌或电子邮件地址。电子邮件地址请使用 @example.invalid（RFC 2606）。

2. 代码质量

代码是否整洁、符合惯用写法且易于维护？

• Go 惯用写法：遵循 Effective Go 和 Go Code Review Comments
• 命名：名称应清晰明确，遵循 Go 惯例（未导出名称使用 camelCase，已导出名称使用 PascalCase），接口名称使用 -er 后缀
• 文件大小：文件应少于 500 行。较大的文件应拆分。
• 导入：标准库在前，其次是第三方库，最后是内部包（github.com/dynatrace-oss/dtctl）
• 重复代码：查找可提取为辅助函数的复制粘贴代码
• 注释：为已导出的函数和类型编写文档注释。注释应解释“为什么”，而不是“是什么”。
• 模式一致性：新代码应遵循代码库中的现有模式。检查 pkg/resources/ 中类似资源的参考实现。

运行 linter，检查肉眼可能忽略的问题：

make lint-strict

3. 用户体验

从用户的角度来看，使用体验是否合适？

• 命令命名：遵循 dtctl <verb> <resource> 模式。不要自定义查询标志——使用 DQL 直通。
• 输出格式：表格输出应易于阅读，列应合理且对齐。JSON/YAML 输出应简洁清晰。
• 错误消息：清晰、可采取行动，并建议后续步骤。在 Agent 模式（-A）下，错误应为包含机器可读代码的结构化 JSON。
• 交互行为：名称解析和消歧提示应正常工作。--plain 会禁用交互行为。
• 帮助文本：命令应包含 Short 描述、Long 描述和 Example 字段。父级动词命令应提供示例。
• 别名：资源应有合理的别名（例如，workflow 使用 wf，dashboard 使用 dash）。
• Agent 模式：命令应支持带上下文建议的 --agent 封装。使用 -A -o json 进行测试。
• 颜色控制：遵循 NO_COLOR、FORCE_COLOR、--plain 和 TTY 检测。

尝试运行实际命令，体验其使用感受：

# Does the help text look good?
dtctl <command> --help

# Does table output look right?
dtctl <command> --plain

# Does agent mode work?
dtctl <command> -A

4. 测试覆盖率

这些更改是否经过了充分测试？

• 单元测试：为新增函数编写测试。优先使用表驱动测试。
• 覆盖率目标：总体最低 70%，新包达到 80%，关键包（pkg/client、pkg/config）达到 90%。
• 边界情况测试：不只是测试正常路径，还要测试错误条件、空输入和边界值。
• 模拟服务防护：分页模拟服务必须拒绝无效的参数组合（例如，page-size + page-key）。Settings API 模拟也必须拒绝同时带有 nextPageKey 的 schemaIds/scopes。
• Golden 测试：如果输出格式发生变化或新增了资源，必须更新 golden 文件。检查：

  go test ./pkg/output/ -run TestGolden
  

  Golden 测试使用 pkg/resources/* 中实际用于生产的结构体，绝不能使用仅供测试的重复定义。
• 端到端测试：在 test/e2e/ 中为新资源或复杂工作流添加集成场景。

# Run full suite
go test ./...

# Check coverage
make test-coverage

5. 文档

是否为用户和贡献者妥善编写了相关文档？

始终需要：

• Conventional Commit 标题：PR 标题（以及 squash merge 提交）必须是有效的 Conventional Commit（feat: ...、fix: ...，破坏性变更使用 feat!: ...）——release-please 会据此确定版本升级幅度并生成 GitHub Release 说明。无需编辑 CHANGELOG.md。

新增功能需要：

• README.md：如果该功能面向用户且影响显著（例如新增资源类型或命令类别），则需更新
• docs/QUICK_START.md：为主要新功能添加使用示例
• docs/dev/IMPLEMENTATION_STATUS.md：更新功能矩阵中的行
• docs/dev/API_DESIGN.md：如果引入了新约定，则记录相应设计模式
• docs/TOKEN_SCOPES.md：如果该功能需要额外的 API 权限，则记录新增的 scope

新增资源需要：

• 资源专属文档页面：放在 docs/resources/ 中，并在 scripts/gen-docs/gen_all.py 中添加 PLAN 条目，让生成器负责管理其中的表格
• 命令参考：重新生成，不要手动编辑——make docs-generate 会重写 docs/COMMANDS.md

新增 AI Agent 支持需要：

• README.md、docs/QUICK_START.md、docs/dev/API_DESIGN.md、docs/dev/IMPLEMENTATION_STATUS.md（全部四个文件）

6. 文档检查

项目没有文档网站：docs/site/ 已弃用，仅保留重定向存根，确保之前发布的 URL 仍可跳转到 docs/。在 docs/site/_docs/ 下新增页面是不正确的。

检查：

• 生成的代码块未被手动修改：不要编辑 `` 内部的内容。如果目录有变化，应运行 make docs-generate 并提交生成结果。
• 新增资源已映射：新的目录资源应出现在 scripts/gen-docs/INDEX.md 的某个页面下，而不是未映射列表中。
• 已验证说明文字：make docs-check 通过——每个文档中列出的 dtctl ... 命令都能解析，且每个标志都存在。CI 会运行此检查，但审阅者应确认新增示例确实运行过，而非凭空编写。
• 链接：相对链接均可解析，锚点均存在（make docs-check 会检查这些内容）。

7. PR 描述

PR 本身是否描述清楚？

• 标题：清晰，遵循 Conventional Commit 风格（feat: ...、fix: ...）
• 摘要：说明更改内容及其原因（而不只是列出修改了哪些文件）
• 相关问题：使用 Closes #NNN 或 Fixes #NNN 引用问题
• 破坏性变更：如有，需明确指出
• 测试部分：说明如何测试这些更改
• UX 示例：对于面向用户的更改，提供 CLI 输出的前后对比

审查结果

完成审查后，请按以下格式提供摘要：

## PR Review: <title>

| Dimension | Status | Notes |
|-----------|--------|-------|
| Production readiness | Pass/Needs work | ... |
| Code quality | Pass/Needs work | ... |
| User experience | Pass/Needs work | ... |
| Test coverage | Pass/Needs work | ... |
| Documentation | Pass/Needs work | ... |
| GitHub Pages | Pass/Needs work/N/A | ... |
| PR description | Pass/Needs work | ... |

### Issues Found
1. **[Dimension]** file:line -- description of issue
2. ...

### Suggestions (non-blocking)
1. ...

### Verdict
Ready to merge / Needs revisions (list blockers)

直接、具体地说明问题。引用准确的文件路径和行号。区分阻塞问题（必须修复）和建议（可选改进）。不要用赞美充字数——只关注需要处理的事项。