Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/controller/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@

- [`addon-controller-diagnostic-pr-scope-guide.md`](addon-controller-diagnostic-pr-scope-guide.md) — controller 诊断-only PR 的边界:只为抓证据加日志时不默认进 main,先走临时分支 / release 分支 / patch image;证据闭合后再开正式修复 PR。
- [`addon-controller-patch-identity-preservation-guide.md`](addon-controller-patch-identity-preservation-guide.md) — controller PR 已被 addon team build 成 patch image 后,review feedback 优先用 follow-up commit 保留 evidence 锚点;必须改写 production SHA 时先通知验证 owner 重核 patch identity。
- [`addon-controller-pr-author-motivation-and-review-channel-guide.md`](addon-controller-pr-author-motivation-and-review-channel-guide.md) — controller PR body 第一段先写具体问题、现场例子和不修会误导什么;second-eye review 的过程意见走团队内部讨论,GitHub PR comment 只留 maintainer 决策需要的信息。
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
# Controller PR: Motivation in Body, Second-Eye Review Outside PR Comments

> **Audience**: KubeBlocks controller / DataProtection / Configure / Ops / lifecycle / syncer 开发, second-eye reviewer, maintainer
> **Status**: stable
> **Applies to**: 所有 controller 仓库 (kubeblocks 主仓 + 相关) 的 PR 提交和 review
> **Applies to KB version**: all
> **Affected by version skew**: yes — 不同 release 分支和 maintainer 队列可能有不同 review 门槛,PR body 必须写清目标分支和现场范围。

controller PR review 常见两个痛点:第一,PR body 只写"改了什么",不写"用户撞到了什么具体问题、现场例子、不修会误导谁",maintainer 看 5 分钟还不知道为啥要 review。第二,second-eye review 的过程性意见(例如"4 个边界都验过"、"本地 8 subtest PASS"、"两边 package diff = 0")被写进 GitHub PR comment,maintainer 要从一堆过程注解里找真正的 blocker。这篇 guide 规定 controller PR 提交和 review 两条硬规则,让 PR body 服务 maintainer 决策,让 second-eye 协作留在团队内部讨论里。

## 先用白话理解这篇文档

简单两条:

1. **PR 开头先写"为啥要改"**。具体是哪个用户撞到了什么问题、用什么例子能复现、不修会让谁被误导。代码细节放后面。
2. **second-eye review 的过程互通**("我跑过了 PASS"、"我看了这 4 条边界")走团队内部讨论。GitHub PR comment 只留给 maintainer 需要看的:blocker、设计决策、最终验证证据。

## 适用场景

| 我在做什么 | 这篇 guide 怎么用 |
|---|---|
| 开新 controller PR | 第一段必写"具体问题 + 现场例子 + 不修会误导什么", 再写代码改动 |
| 做 second-eye review | 结论 + 验证命令 + 关注点走团队内部讨论, 不发到 GH PR comment |
| GH PR comment 该写什么 | maintainer 决策需要的: blocker / 设计决策 / 最终验证证据 / patch identity 同源核对结果 |

## 通用方法论

### 硬规则

1. **PR body 第一段必须写**:
- 具体问题 (一句话: 某个 addon team 撞到 X)
- 现场例子 (具体值 / 错误信息 / 失败链路, 不是"一种典型场景")
- 不修会误导什么 (用户/测试系统会怎么被假象骗到)
2. **后续段落再写**: scope (改动范围), why (原理), verification (本地跑了什么), related (相关 PR / issue)
3. **second-eye review 走团队内部讨论**: 协作过程 (本地跑了什么测试, 边界验过哪几条, 双 package diff 是否相同) 不发 GH PR comment
4. **GH PR comment 留三类**: (a) maintainer 决策需要的设计 blocker, (b) author 自己的 commit 进度更新 (例 "follow-up commit `<sha>` 加 negative 测试"), (c) 最终验证证据 (例 "patch identity `<sha>` 核对 PASS")
5. **不在 PR comment 里追问 / 协商**: 比如"你这个 boundary 是不是太严", "我能不能改成 X" — 走团队内部讨论
6. **已发的 review-style PR comment 不撤回**: 撤回反而显眼 + GitHub audit log 改不掉, 留作历史; 但不再追加新的

### PR body 模板

```markdown
## 具体问题

某个 addon (例: Valkey) 撞到 X 行为, 期望 Y 但实际 Z。

现场例子: <具体值 / 错误信息 / 失败链路>

不修会误导: <用户看到 ... 实际上 ...>; <测试系统标 ... 实际 ...>

## Scope

<改了哪些 package, 改了什么>

## Why

<原理简介, 设计选择>

## Verification

<本地 / CI 跑过什么命令>

## Related

<相关 PR / issue / 沉淀文档>
```

### Review channel 决策树

| review 内容 | 走哪 |
|---|---|
| "我跑过 `go test ...` PASS" | 团队内部讨论 |
| "我看了你这 4 条边界, 都验过了" | 团队内部讨论 |
| "Boundary X 我建议改成 Y 因为 Z" (协商) | 团队内部讨论, 商定后 author 自己在 PR body 或 commit message 里写最终设计决策 |
| "blocker: 你 Ops 包做太重, 校验应该放 parameters" (架构 blocker) | GH PR comment (maintainer 决策需要看) |
| "follow-up commit `<sha>` 加 N case" (author 进度) | GH PR comment |
| "patch identity `<old>` vs `<new>` 同源核对结果" (最终验证证据) | GH PR comment |

### 反模式表

| 反模式 | 后果 | 修法 |
|---|---|---|
| PR body 第一段只写"改 `pkg/X/Y.go` 加 Z 函数" | maintainer 不知道 motivation, review 不动 | 第一段写问题 / 例子 / 误导 |
| second-eye review 全发 GH PR comment | maintainer 看不到真正 blocker, review 噪音 | 走团队内部讨论, GH 只留 blocker |
| PR comment 里和 reviewer 来回协商 | maintainer 看不下来 | 团队内部协商, 最终决策写进 PR body / commit message |
| "现场例子" 写成 "通常会出现" / "在某些场景下" | 抽象, maintainer 没法快速判断范围 | 给具体值 / 错误信息 / 测试链路 |

## 与其他 doc / skill 的关系

- [`addon-controller-patch-identity-preservation-guide.md`](addon-controller-patch-identity-preservation-guide.md) — PR head SHA 保 patch identity 锚点; PR comment 该写什么 vs 走团队内部讨论的边界一致
- [`../code-submission/addon-github-submission-discipline-guide.md`](../code-submission/addon-github-submission-discipline-guide.md) — 通用 PR / commit hygiene
- [`../code-submission/addon-github-pr-merge-gate-semantics-guide.md`](../code-submission/addon-github-pr-merge-gate-semantics-guide.md) — merge gate 决定 PR comment 哪些是真正的 blocker

## 案例附录

2026-05-19 的一个 controller PR case 触发了两条规则:
1. 用户实际撞到的问题应该写在 PR 开头, 作为 motivation。
2. second-eye review 的过程意见留在团队内部讨论, 不写进 GitHub PR comment。

触发现场: PR body 只写"修法原理 + verification", 没有开门见山讲"非法改参数请求被控制面误报成功"。second-eye review 意见 (4 边界都验过 / 本地 8 subtest PASS / patch identity 同源) 之前都发到了 GH PR comment, 导致 maintainer 要从多条过程注解里挑真正的 blocker。

修法: PR body 加"具体问题 + 现场例子 + 不修误导"开头, second-eye review 走团队内部讨论, GH PR comment 仅留 maintainer 需要的 blocker / 设计决策 / 最终验证证据。

## 一句话总结

controller PR body 第一段写 "具体问题 + 现场例子 + 不修会误导什么", second-eye review 走团队内部讨论不发 GH PR comment, GH PR comment 只留 maintainer 决策需要的 blocker / 设计决策 / 最终验证证据。