From 9d63f8c200e6ad8d89527407a1780aaac59ed4ff Mon Sep 17 00:00:00 2001 From: Wei Cao Date: Tue, 19 May 2026 13:06:55 +0800 Subject: [PATCH 1/2] docs(controller): add PR motivation and second-eye review channel guide --- ...hor-motivation-and-review-channel-guide.md | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 docs/controller/addon-controller-pr-author-motivation-and-review-channel-guide.md diff --git a/docs/controller/addon-controller-pr-author-motivation-and-review-channel-guide.md b/docs/controller/addon-controller-pr-author-motivation-and-review-channel-guide.md new file mode 100644 index 0000000..afa7685 --- /dev/null +++ b/docs/controller/addon-controller-pr-author-motivation-and-review-channel-guide.md @@ -0,0 +1,106 @@ +# Controller PR: Motivation in Body, Second-Eye Review in DM + +> **Audience**: KubeBlocks controller / DP / Configure / Ops / lifecycle / syncer 开发, XP pair reviewer, maintainer +> **Status**: stable +> **Applies to**: 所有 controller 仓库 (kubeblocks 主仓 + 相关) 的 PR 提交和 review +> **Applies to KB version**: any +> **Affected by version skew**: no + +controller PR review 这周连撞两个痛点：第一，PR body 只写"改了什么"，不写"用户撞到了什么具体问题、现场例子、不修会误导谁"，maintainer 看 5 分钟还不知道为啥要 review，时间被消耗；第二，XP pair 的 second-eye review 意见（"4 个边界都验过"、"本地 8 subtest PASS"、"两边 pkg diff = 0"）被写进 GitHub PR comment，把内部协作过程暴露成公共评论流，maintainer 要从一堆 second-eye 注解里找真正的 blocker。这篇 guide 规定 controller PR 提交和 review 两条硬规则，让 PR body 服务 maintainer 决策，让 second-eye 协作走 DM / Slock thread。 + +## 先用白话理解这篇文档 + +简单两条： + +1. **PR 开头先写"为啥要改"**。具体是哪个用户撞到了什么问题、用什么例子能复现、不修会让谁被误导。代码细节放后面。 +2. **XP pair 之间的 review 互通**（"我跑过了 PASS"、"我看了这 4 条边界"）走 DM 或 Slock thread。GitHub PR comment 只留给 maintainer 需要看的：blocker、设计决策、最终验证证据。 + +## 适用场景 + +| 我在做什么 | 这篇 guide 怎么用 | +|---|---| +| 开新 controller PR | 第一段必写"具体问题 + 现场例子 + 不修会误导什么", 再写代码改动 | +| 做 XP pair second-eye review | 结论 + 验证命令 + 关注点走 DM / Slock thread, 不发到 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 走 DM / Slock thread**: 协作过程 (本地跑了什么测试, 边界验过哪几条, 双 pkg diff 是否相同) 不发 GH PR comment +4. **GH PR comment 留三类**: (a) maintainer 决策需要的设计 blocker, (b) author 自己的 commit 进度更新 (例 "follow-up commit `` 加 negative 测试"), (c) 最终验证证据 (例 "Bob2 r15-patch round-1 commit `` patch identity 核对 PASS") +5. **不在 PR comment 里追问 / 协商**: 比如"你这个 boundary 是不是太严", "我能不能改成 X" — 走 DM +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" | DM | +| "我看了你这 4 条边界, 都验过了" | DM | +| "Boundary X 我建议改成 Y 因为 Z" (协商) | DM, 商定后 author 自己在 PR body 或 commit message 里写最终设计决策 | +| "blocker: 你 Ops 包做太重, 校验应该放 parameters" (架构 blocker) | GH PR comment (maintainer 决策需要看) | +| "follow-up commit `` 加 N case" (author 进度) | GH PR comment | +| "patch identity `` vs `` 同源核对结果" (最终验证证据) | GH PR comment | + +### 反模式表 + +| 反模式 | 后果 | 修法 | +|---|---|---| +| PR body 第一段只写"改 `pkg/X/Y.go` 加 Z 函数" | maintainer 不知道 motivation, review 不动 | 第一段写问题 / 例子 / 误导 | +| second-eye review 全发 GH PR comment | maintainer 看不到真正 blocker, review 噪音 | 走 DM, GH 只留 blocker | +| PR comment 里和 reviewer 来回协商 | maintainer 看不下来 | DM 协商, 最终决策写进 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 走 DM 的边界一致 +- [`../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 西方政策触发: westonnnn 在 PR #10254 thread (msg 092c86d6 + 5cd7ef7d) 给两条: +1. "用户实际撞到的问题应该写在 PR 开头, 作为 motivation" +2. "不要去写 second-eye review 意见, 你们之间 DM 就好" + +触发现场: PR #10254 Lily author body 只写"修法原理 + verification", 不开门见山讲"非法改参数请求被控制面误报成功"。XP pair (Edward + Rocco) second-eye review 意见 (4 边界都验过 / 本地 8 subtest PASS / patch identity 同源) 之前都发到了 GH PR comment, 导致 maintainer 要从多条 second-eye 注解里挑真正的 blocker。 + +修法 (Lily 03baf62ec + Edward a48af7d4 + Rocco 294f6085 三方齐 ack): PR body 加"具体问题 + 现场例子 + 不修误导"开头, second-eye review 走 DM, GH PR comment 仅留 maintainer 需要的 blocker / 设计决策 / 最终验证证据。 + +## 一句话总结 + +controller PR body 第一段写 "具体问题 + 现场例子 + 不修会误导什么", XP pair second-eye review 走 DM / Slock thread 不发 GH PR comment, GH PR comment 只留 maintainer 决策需要的 blocker / 设计决策 / 最终验证证据。 From 38b4d5aa0155248b28875f7f37e025e14b8bb9d5 Mon Sep 17 00:00:00 2001 From: Wei Cao Date: Tue, 19 May 2026 13:34:20 +0800 Subject: [PATCH 2/2] docs(controller): polish PR motivation guide --- docs/controller/README.md | 1 + ...hor-motivation-and-review-channel-guide.md | 44 +++++++++---------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/docs/controller/README.md b/docs/controller/README.md index 4eff4c7..6ee6726 100644 --- a/docs/controller/README.md +++ b/docs/controller/README.md @@ -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 决策需要的信息。 diff --git a/docs/controller/addon-controller-pr-author-motivation-and-review-channel-guide.md b/docs/controller/addon-controller-pr-author-motivation-and-review-channel-guide.md index afa7685..809b5f0 100644 --- a/docs/controller/addon-controller-pr-author-motivation-and-review-channel-guide.md +++ b/docs/controller/addon-controller-pr-author-motivation-and-review-channel-guide.md @@ -1,26 +1,26 @@ -# Controller PR: Motivation in Body, Second-Eye Review in DM +# Controller PR: Motivation in Body, Second-Eye Review Outside PR Comments -> **Audience**: KubeBlocks controller / DP / Configure / Ops / lifecycle / syncer 开发, XP pair reviewer, maintainer +> **Audience**: KubeBlocks controller / DataProtection / Configure / Ops / lifecycle / syncer 开发, second-eye reviewer, maintainer > **Status**: stable > **Applies to**: 所有 controller 仓库 (kubeblocks 主仓 + 相关) 的 PR 提交和 review -> **Applies to KB version**: any -> **Affected by version skew**: no +> **Applies to KB version**: all +> **Affected by version skew**: yes — 不同 release 分支和 maintainer 队列可能有不同 review 门槛，PR body 必须写清目标分支和现场范围。 -controller PR review 这周连撞两个痛点：第一，PR body 只写"改了什么"，不写"用户撞到了什么具体问题、现场例子、不修会误导谁"，maintainer 看 5 分钟还不知道为啥要 review，时间被消耗；第二，XP pair 的 second-eye review 意见（"4 个边界都验过"、"本地 8 subtest PASS"、"两边 pkg diff = 0"）被写进 GitHub PR comment，把内部协作过程暴露成公共评论流，maintainer 要从一堆 second-eye 注解里找真正的 blocker。这篇 guide 规定 controller PR 提交和 review 两条硬规则，让 PR body 服务 maintainer 决策，让 second-eye 协作走 DM / Slock thread。 +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. **XP pair 之间的 review 互通**（"我跑过了 PASS"、"我看了这 4 条边界"）走 DM 或 Slock thread。GitHub PR comment 只留给 maintainer 需要看的：blocker、设计决策、最终验证证据。 +2. **second-eye review 的过程互通**（"我跑过了 PASS"、"我看了这 4 条边界"）走团队内部讨论。GitHub PR comment 只留给 maintainer 需要看的：blocker、设计决策、最终验证证据。 ## 适用场景 | 我在做什么 | 这篇 guide 怎么用 | |---|---| | 开新 controller PR | 第一段必写"具体问题 + 现场例子 + 不修会误导什么", 再写代码改动 | -| 做 XP pair second-eye review | 结论 + 验证命令 + 关注点走 DM / Slock thread, 不发到 GH PR comment | +| 做 second-eye review | 结论 + 验证命令 + 关注点走团队内部讨论, 不发到 GH PR comment | | GH PR comment 该写什么 | maintainer 决策需要的: blocker / 设计决策 / 最终验证证据 / patch identity 同源核对结果 | ## 通用方法论 @@ -32,9 +32,9 @@ controller PR review 这周连撞两个痛点：第一，PR body 只写"改了 - 现场例子 (具体值 / 错误信息 / 失败链路, 不是"一种典型场景") - 不修会误导什么 (用户/测试系统会怎么被假象骗到) 2. **后续段落再写**: scope (改动范围), why (原理), verification (本地跑了什么), related (相关 PR / issue) -3. **second-eye review 走 DM / Slock thread**: 协作过程 (本地跑了什么测试, 边界验过哪几条, 双 pkg diff 是否相同) 不发 GH PR comment -4. **GH PR comment 留三类**: (a) maintainer 决策需要的设计 blocker, (b) author 自己的 commit 进度更新 (例 "follow-up commit `` 加 negative 测试"), (c) 最终验证证据 (例 "Bob2 r15-patch round-1 commit `` patch identity 核对 PASS") -5. **不在 PR comment 里追问 / 协商**: 比如"你这个 boundary 是不是太严", "我能不能改成 X" — 走 DM +3. **second-eye review 走团队内部讨论**: 协作过程 (本地跑了什么测试, 边界验过哪几条, 双 package diff 是否相同) 不发 GH PR comment +4. **GH PR comment 留三类**: (a) maintainer 决策需要的设计 blocker, (b) author 自己的 commit 进度更新 (例 "follow-up commit `` 加 negative 测试"), (c) 最终验证证据 (例 "patch identity `` 核对 PASS") +5. **不在 PR comment 里追问 / 协商**: 比如"你这个 boundary 是不是太严", "我能不能改成 X" — 走团队内部讨论 6. **已发的 review-style PR comment 不撤回**: 撤回反而显眼 + GitHub audit log 改不掉, 留作历史; 但不再追加新的 ### PR body 模板 @@ -69,9 +69,9 @@ controller PR review 这周连撞两个痛点：第一，PR body 只写"改了 | review 内容 | 走哪 | |---|---| -| "我跑过 `go test ...` PASS" | DM | -| "我看了你这 4 条边界, 都验过了" | DM | -| "Boundary X 我建议改成 Y 因为 Z" (协商) | DM, 商定后 author 自己在 PR body 或 commit message 里写最终设计决策 | +| "我跑过 `go test ...` PASS" | 团队内部讨论 | +| "我看了你这 4 条边界, 都验过了" | 团队内部讨论 | +| "Boundary X 我建议改成 Y 因为 Z" (协商) | 团队内部讨论, 商定后 author 自己在 PR body 或 commit message 里写最终设计决策 | | "blocker: 你 Ops 包做太重, 校验应该放 parameters" (架构 blocker) | GH PR comment (maintainer 决策需要看) | | "follow-up commit `` 加 N case" (author 进度) | GH PR comment | | "patch identity `` vs `` 同源核对结果" (最终验证证据) | GH PR comment | @@ -81,26 +81,26 @@ controller PR review 这周连撞两个痛点：第一，PR body 只写"改了 | 反模式 | 后果 | 修法 | |---|---|---| | PR body 第一段只写"改 `pkg/X/Y.go` 加 Z 函数" | maintainer 不知道 motivation, review 不动 | 第一段写问题 / 例子 / 误导 | -| second-eye review 全发 GH PR comment | maintainer 看不到真正 blocker, review 噪音 | 走 DM, GH 只留 blocker | -| PR comment 里和 reviewer 来回协商 | maintainer 看不下来 | DM 协商, 最终决策写进 PR body / commit message | +| 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 走 DM 的边界一致 +- [`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 西方政策触发: westonnnn 在 PR #10254 thread (msg 092c86d6 + 5cd7ef7d) 给两条: -1. "用户实际撞到的问题应该写在 PR 开头, 作为 motivation" -2. "不要去写 second-eye review 意见, 你们之间 DM 就好" +2026-05-19 的一个 controller PR case 触发了两条规则: +1. 用户实际撞到的问题应该写在 PR 开头, 作为 motivation。 +2. second-eye review 的过程意见留在团队内部讨论, 不写进 GitHub PR comment。 -触发现场: PR #10254 Lily author body 只写"修法原理 + verification", 不开门见山讲"非法改参数请求被控制面误报成功"。XP pair (Edward + Rocco) second-eye review 意见 (4 边界都验过 / 本地 8 subtest PASS / patch identity 同源) 之前都发到了 GH PR comment, 导致 maintainer 要从多条 second-eye 注解里挑真正的 blocker。 +触发现场: PR body 只写"修法原理 + verification", 没有开门见山讲"非法改参数请求被控制面误报成功"。second-eye review 意见 (4 边界都验过 / 本地 8 subtest PASS / patch identity 同源) 之前都发到了 GH PR comment, 导致 maintainer 要从多条过程注解里挑真正的 blocker。 -修法 (Lily 03baf62ec + Edward a48af7d4 + Rocco 294f6085 三方齐 ack): PR body 加"具体问题 + 现场例子 + 不修误导"开头, second-eye review 走 DM, GH PR comment 仅留 maintainer 需要的 blocker / 设计决策 / 最终验证证据。 +修法: PR body 加"具体问题 + 现场例子 + 不修误导"开头, second-eye review 走团队内部讨论, GH PR comment 仅留 maintainer 需要的 blocker / 设计决策 / 最终验证证据。 ## 一句话总结 -controller PR body 第一段写 "具体问题 + 现场例子 + 不修会误导什么", XP pair second-eye review 走 DM / Slock thread 不发 GH PR comment, GH PR comment 只留 maintainer 决策需要的 blocker / 设计决策 / 最终验证证据。 +controller PR body 第一段写 "具体问题 + 现场例子 + 不修会误导什么", second-eye review 走团队内部讨论不发 GH PR comment, GH PR comment 只留 maintainer 决策需要的 blocker / 设计决策 / 最终验证证据。