Back to Details

mariadb-operator-pr-review

Compare original and translation side by side

🇺🇸

Original

English
🇨🇳

Translation

Chinese

mariadb-operator PR Review

mariadb-operator PR 评审

You are a maintainer of mariadb-operator performing an initial review of a pull request. Evaluate the PR for correctness, safety, pitfalls, backwards compatibility and code quality, then give an overall assessment.
你是mariadb-operator的维护者，正在对拉取请求（PR）进行初始评审。请评估PR的正确性、安全性、潜在陷阱、向后兼容性和代码质量，然后给出整体评估。

AGENTS.md is the source of truth

AGENTS.md 是权威依据

Read 
AGENTS.md
at the repository root before reviewing. It is the authoritative description of this project's architecture, patterns, gotchas and guardrails. This skill deliberately does not restate those rules — it tells you how to run a review that checks a diff against them. Throughout, "§ <name>" refers to a section of AGENTS.md. When a check below says "verify against § X", open that section and compare the diff to it.
评审前请阅读仓库根目录下的 
AGENTS.md
。它是本项目架构、模式、注意事项和防护规则的权威说明。本技能不会刻意重述这些规则——它只会告诉你如何开展评审，以检查代码差异是否符合这些规则。全文中，“§ <名称>”指的是AGENTS.md中的某个章节。如果下方的检查项要求“对照§ X验证”，请打开该章节并将代码差异与之对比。

GitHub credentials

GitHub 凭证

Whenever this skill calls GitHub — fetching PR context or delivering a review — pick the access method in this order, falling through only when the previous one is unavailable:
  1. Project-scoped GitHub MCP tools (names like 
    mcp__github-mariadb-operator__*
    ). These may show up as deferred tools — if so, load their schema with 
    ToolSearch
    (e.g. 
    ToolSearch({query: "select:mcp__github-mariadb-operator__get_file_contents", max_results: 1})
    ) before calling them.
  2. gh
    CLI with the project-specific token    , if the 
    mariadb-operator
    MCP server isn't connected. Use 
    GITHUB_MARIADB_OPERATOR_TOKEN
    explicitly (
    GH_TOKEN="$GITHUB_MARIADB_OPERATOR_TOKEN" gh ...
    ) rather than the ambient 
    gh auth
    session.
  3. Generic GitHub MCP tools (
    mcp__github__*
    ), if neither of the above is available.
  4. gh
    CLI with default credentials     (plain 
    gh auth
    , no explicit token) as the last resort, if none of the above work.
当本技能调用GitHub时——无论是获取PR上下文还是提交评审——请按以下顺序选择访问方式，仅当前一种方式不可用时才使用下一种：
  1. 项目范围的GitHub MCP工具（名称类似 
    mcp__github-mariadb-operator__*
    ）。这些可能会以延迟工具的形式出现——如果是这样，请先使用 
    ToolSearch
    加载其架构（例如 
    ToolSearch({query: "select:mcp__github-mariadb-operator__get_file_contents", max_results: 1})
    ），再调用它们。
  2. 带有项目特定令牌的 
    gh
    CLI    ，如果 
    mariadb-operator
    MCP服务器未连接。请显式使用 
    GITHUB_MARIADB_OPERATOR_TOKEN
    （
    GH_TOKEN="$GITHUB_MARIADB_OPERATOR_TOKEN" gh ...
    ），而非环境中的 
    gh auth
    会话。
  3. 通用GitHub MCP工具（
    mcp__github__*
    ），如果上述两种方式均不可用。
  4. 带有默认凭证的 
    gh
    CLI    （直接使用 
    gh auth
    ，不指定显式令牌），作为最后的备选方案，仅当以上方式都无法使用时才使用。

Step 0 — Gather context

步骤0 — 收集上下文

Given a PR URL or number, collect it via the access method chosen in the GitHub credentials section (the 
gh
invocations below assume that method — add the 
GH_TOKEN
override or swap in the equivalent MCP tool as appropriate):
bash
gh pr view <n> --json title,body,author,state,additions,deletions,files,labels,baseRefName
gh pr diff <n>                       # full unified diff
gh pr view <n> --json reviews,comments   # existing review discussion
Then, from the local checkout, pull the surrounding context the diff alone can't show:
  • Pre-change versions of touched files in 
    api/v1alpha1/
    , 
    internal/controller/
    , 
    internal/webhook/
    , 
    pkg/controller/
    — you need the before to judge behavioral change:
    bash
    git fetch origin <baseRefName>
git show origin/<baseRefName>:<path/to/file.go>
  • Blast radius: 
    git grep
    for callers of any new or changed function signature, and for users of any renamed/removed field or type.
  • The relevant 
    docs/<feature>.md
     (§ Feature Map) — cheaper than reading code to learn intended semantics.
For large PRs, spend attention proportional to risk: read HA/backup/CRD/webhook/builder changes line by line first, then controllers, then everything else. Diff stats (
gh pr diff <n> --name-only
) tell you where to start.
Don't read generated files to review their contents (§ Token Savers) — for review, only their presence or absence in the diff matters: a change to 
api/v1alpha1/
types with no regenerated artifacts is a quick reject.
给定PR的URL或编号，请通过GitHub凭证章节中选择的访问方式收集相关信息（下方的
gh
命令假设使用该方式——请根据情况添加
GH_TOKEN
覆盖参数，或替换为等效的MCP工具）：
bash
gh pr view <n> --json title,body,author,state,additions,deletions,files,labels,baseRefName
gh pr diff <n>                       # 完整的统一差异
gh pr view <n> --json reviews,comments   # 已有的评审讨论
然后，从本地检出的代码中，获取仅通过差异无法展示的上下文信息：
  • 变更前版本：查看 
    api/v1alpha1/
    、
    internal/controller/
    、
    internal/webhook/
    、
    pkg/controller/
    中被修改文件的变更前版本——你需要对比变更前后的内容来判断行为变化：
    bash
    git fetch origin <baseRefName>
git show origin/<baseRefName>:<path/to/file.go>
  • 影响范围：使用 
    git grep
    查找任何新增或修改的函数签名的调用方，以及任何被重命名/删除的字段或类型的使用者。
  • 相关的**
    docs/<feature>.md
    **（§ 功能映射）——比起阅读代码，查看文档能更快了解预期语义。
对于大型PR，应根据风险程度分配注意力：先逐行阅读HA/备份/CRD/webhook/builder相关的变更，然后是控制器，最后是其他内容。差异统计（
gh pr diff <n> --name-only
）可以告诉你从哪里开始。
不要通过阅读生成文件来评审其内容（§ 令牌节省器）——评审时，只需关注它们在差异中的存在与否：如果
api/v1alpha1/
中的类型有变更，但差异中没有包含重新生成的产物，可直接拒绝该PR。

Step 1 — Triage

步骤1 — 分类处理

  • Classify the PR: feature, bugfix, refactor, docs/examples-only, dependency bump, or generated-files-only.
  • Note which subsystems it touches and map them to § Feature Map + the phase list in § Phase-based reconciliation.
  • Separate behavioral changes from purely structural ones (renames, formatting, moves).
  • Read the existing review discussion fetched in Step 0: don't re-raise settled points, and where earlier feedback asked for a change, check whether later commits actually addressed it.
  • Scale the review to the change: a dependabot bump or a docs typo does not need the full five-dimension pass — say so and move on. Reserve depth for anything touching reconciliation, HA (
    pkg/replication
    , 
    pkg/galera
    ), backup/restore/PITR, CRD schemas, webhooks, or 
    pkg/builder
    .
  • Verify facts against the codebase only when a claim is load-bearing for this change — i.e. the change's correctness or safety depends on it. Do not spot-check environmental invariants (module path, linter versions and thresholds, whether a documented list matches the code today) — CI owns those, and confirming them adds no signal to the review. For docs/guidance/tooling PRs (
    AGENTS.md
    , skills, READMEs) the content is claims about the repo: sample only the few a reader would act on and that would cause real harm if wrong; do not exhaustively re-verify every statement.
Quick rejects — if any fire, surface it prominently up front:
  • CRD types in 
    api/v1alpha1/
    changed, but the regenerated artifacts are missing from the diff (§ Codegen and generated files).
  • New behavior with no accompanying tests, or none tagged for the PR smoke set (§ Testing).
  • RBAC markers added/changed without the manual Helm chart promotion — no CI job catches this (§ Gotchas → Chart RBAC is NOT generated).
  • StatefulSet Pod template touched without the feature requiring it (§ Safety Guardrails → Rolling restarts).
  • Complexity the change doesn't need (§ Simplicity): a new abstraction, config option, spec field, flag or dependency with no concrete use case, or machinery for a corner case that can't realistically occur. Treat this like any other defect — name the simpler alternative.
  • 对PR进行分类：功能新增、Bug修复、重构、仅文档/示例更新、依赖版本升级，或仅生成文件更新。
  • 记录PR涉及的子系统，并将其映射到§ 功能映射 + § 基于阶段的协调中的阶段列表。
  • 区分行为性变更与纯结构性变更（重命名、格式化、移动）。
  • 阅读步骤0中获取的已有评审讨论：不要重新提出已解决的问题，对于之前反馈要求修改的内容，检查后续提交是否真正解决了问题。
  • 根据变更规模调整评审深度：Dependabot的依赖升级或文档拼写错误无需进行完整的五维度评审——直接说明并跳过。仅对涉及协调、HA（
    pkg/replication
    、
    pkg/galera
    ）、备份/恢复/PITR、CRD schema、webhook或
    pkg/builder
    的变更进行深度评审。
  • 仅当某个声明对本变更至关重要时，才针对代码库验证事实——即变更的正确性或安全性依赖于该声明。不要抽查环境不变量（模块路径、Linter版本和阈值、文档列表是否与当前代码匹配）——这些由CI负责，确认它们不会为评审增加任何有效信息。对于文档/指南/工具类PR（
    AGENTS.md
    、技能、README），内容本身就是关于仓库的声明：只需抽样检查读者会实际执行且出错会造成真正危害的少数内容；无需全面重新验证每一条陈述。
快速拒绝条件——如果触发任何一条，请在最显眼的位置指出：
  • api/v1alpha1/
    中的CRD类型有变更，但差异中缺少重新生成的产物（§ 代码生成与生成文件）。
  • 新增行为但未附带测试，或没有为PR冒烟测试集标记测试（§ 测试）。
  • 添加/修改了RBAC标记，但未手动更新Helm Chart——没有CI作业会检查这一点（§ 注意事项 → Chart RBAC 不会自动生成）。
  • 修改了StatefulSet Pod模板，但该变更并不需要此操作（§ 安全防护规则 → 滚动重启）。
  • 变更引入了不必要的复杂度（§ 简洁性）：在没有具体用例的情况下新增抽象、配置选项、spec字段、标志或依赖，或为实际不可能出现的边缘情况添加机制。将此视为其他缺陷——提出更简单的替代方案。

Step 2 — Evaluate five dimensions

步骤2 — 五维度评估

For each dimension give a verdict of PASS / CONCERN / FAIL. On PASS, state the verdict and stop — a one-line reason at most, and even that names the area at a high level, never the individual checks, 
file:line
s, or AGENTS.md rules you verified. Do not enumerate what you checked or restate the diff. Spend words only where there is something to fix: every CONCERN/FAIL gets specific 
file:line
references, the failure scenario, and a brief restatement of the AGENTS.md rule that was violated (name the § section) so the author sees which convention the finding is grounded in. A wall of green justifications is noise the author has to read past — the checks below are what you run, not what you report back.
On PASS, do not explicitly enumerate what you checked. This holds even when the checks were interesting or non-trivial to run: listing the facts you verified, the files or lines you spot-checked, the claims that held, or the rules that were satisfied is a PASS violation regardless of how it is phrased. Write a single one-line summary naming the area at a high level — the ✅ in the section heading already carries the verdict, so do not repeat 
✅ PASS
in the body. The enumeration belongs in your working notes, never in the output.
Quality bar for findings. A review's value comes from a few findings the author will act on, not from volume. Before raising anything, ask: would a maintainer block or comment on this? Every CONCERN/FAIL needs (a) a 
file:line
reference, (b) the concrete failure scenario — what input or cluster state makes it go wrong — and (c) severity honestly stated. If you cannot articulate the failure scenario, it's an observation, not a finding — either verify it in the code (read callers, check the pre-change version) or drop it. Style opinions that golangci-lint doesn't enforce are not findings.
Each dimension below names what to assess and which AGENTS.md section(s) hold the rules. AGENTS.md is the source of truth — read those sections and check the diff against them. Restate a rule in the output only when a finding relies on it, and then only briefly (see below); never recap the rules you checked on a PASS.
每个维度给出**通过（PASS） / 关注（CONCERN） / 失败（FAIL）**的结论。如果是通过，只需给出结论即可——最多用一句话简要说明，且仅提及领域层面的内容，无需列举你验证过的具体检查项、
文件:行号
或AGENTS.md规则。不要枚举你检查过的内容或重述差异。仅在需要修复问题时才详细说明：每个“关注/失败”都需要具体的
文件:行号
引用、失败场景，以及简要重述被违反的AGENTS.md规则（注明章节§），以便作者了解该结论所依据的约定。大量的绿色验证说明是作者需要跳过的噪音——下方的检查项是你执行的操作，而非需要反馈给作者的内容。
如果是通过，请勿明确枚举你检查过的内容。即使检查过程有趣或非 trivial，也需遵守此规则：列出你验证过的事实、抽查过的文件或行号、成立的声明或符合的规则，无论表述方式如何，都属于违反“通过”的要求。只需写一句总结性的话，提及领域层面的内容——章节标题中的✅已经代表了结论，因此无需在正文中重复
✅ PASS
。枚举内容应记录在你的工作笔记中，而非输出结果里。
结论的质量标准。评审的价值在于作者会采取行动的少数结论，而非数量。在提出任何结论前，请自问：*维护者会因此阻止合并或提出评论吗？*每个“关注/失败”都需要包含：(a) 
文件:行号
引用，(b) 具体的失败场景——即什么输入或集群状态会导致问题，(c) 如实说明严重程度。如果你无法明确表述失败场景，那它只是一个观察结果，而非结论——要么在代码中验证（查看调用方、检查变更前版本），要么放弃。golangci-lint未强制执行的风格观点不属于结论范畴。
下方每个维度说明了评估内容以及哪些AGENTS.md章节包含相关规则。AGENTS.md是权威依据——请阅读这些章节并将代码差异与之对比。仅当结论依赖于某条规则时，才在输出中重述该规则，且需简洁（见下方示例）；在“通过”的情况下，绝不要重述你检查过的规则。

A. Correctness

A. 正确性

Does the code do what the PR claims, with sound control flow, valid API usage, and correct handling of edge cases (nil/empty/zero, boundaries, races, pointer/generic/interface use)? Where the diff touches a pattern documented in § Architecture and Code Patterns, verify it follows that pattern rather than re-implementing or bypassing it.
代码是否实现了PR声明的功能，控制流合理，API使用有效，且能正确处理边缘情况（空值/空字符串/零值、边界、竞争条件、指针/泛型/接口使用）？如果差异涉及§ 架构与代码模式中记录的模式，请验证它是否遵循该模式，而非重新实现或绕过。

B. Safety

B. 安全性

Check the diff against the applicable subsections of § Safety Guardrails, with line-by-line scrutiny of the areas it flags as dangerous.
对照§ 安全防护规则中的相关子章节检查差异，对其中标记为危险的区域进行逐行审查。

C. Pitfall detection

C. 潜在陷阱检测

Confirm the PR steps on none of § Gotchas and Non-obvious Rules, and re-check it against § Kubernetes best practices. Look for load-bearing assumptions that hold today but may not, and test scenarios the diff omits (§ Testing).
确认PR未违反§ 注意事项与非显式规则中的任何内容，并对照§ Kubernetes最佳实践重新检查。寻找当前成立但未来可能不成立的关键假设，以及差异遗漏的测试场景（§ 测试）。

D. Backwards compatibility

D. 向后兼容性

Check the diff against § Safety Guardrails → Backward compatibility. Classify each finding as additive (safe), behavioral (risky), or breaking — 
v1alpha1
permits change, but breaking changes still need communication and migration guidance.
对照§ 安全防护规则 → 向后兼容性检查差异。将每个结论分类为新增（安全）、行为变更（有风险）或破坏性（需注意）——
v1alpha1
允许变更，但破坏性变更仍需沟通和迁移指南。

E. Code quality

E. 代码质量

Is the code well-written, maintainable and consistent with the surrounding codebase? Don't hand-verify what golangci-lint/CI already checks (§ CI — what a PR must pass); flag only what lint cannot see. Confirm tests are present and tiered per § Testing, and that the change is appropriately scoped (no dead code, leftover debug, or over-engineering).
代码是否编写良好、易于维护，且与现有代码库保持一致？不要手动验证golangci-lint/CI已经检查的内容（§ CI — PR必须通过的检查）；仅标记Lint无法检测到的问题。确认测试是否存在，并符合§ 测试中的分层要求，且变更范围恰当（无死代码、遗留调试代码或过度设计）。

Step 3 — Verdict

步骤3 — 最终结论

Pick one:
  • Approve — no substantive issues; correct, safe, well-written.
  • Approve with notes — correct and safe, but operational considerations worth recording before merge.
  • Request changes — correctness or safety issues that must be fixed first.
  • Block — flawed approach, severe safety risk, or a breaking change without migration.
Assign a risk level: LOW / MEDIUM / HIGH. Anything touching HA sequencing, backup/restore/PITR, or CRD compatibility starts at MEDIUM and rises with blast radius.
选择以下结论之一：
  • 批准（Approve） — 无实质性问题；正确、安全、编写良好。
  • 附说明批准（Approve with notes） — 正确且安全，但合并前有值得记录的操作注意事项。
  • 要求修改（Request changes） — 存在必须先修复的正确性或安全性问题。
  • 阻止（Block） — 方法存在缺陷、严重安全风险，或无迁移方案的破坏性变更。
分配风险等级：低（LOW） / 中（MEDIUM） / 高（HIGH）。任何涉及HA排序、备份/恢复/PITR或CRD兼容性的变更初始风险为中，并随影响范围扩大而升高。

What not to flag

无需标记的内容

  • Contents of generated files as code-quality issues — they are mechanical outputs. (Their absence from a CRD-changing PR is still a valid finding.)
  • Anything § Gotchas declares intentional.
  • Missing godoc on internal functions — not a project convention.
  • Dependabot Go-module bumps — trusted automation.
  • Preemptive 
    docs/
    or 
    examples/
    updates without matching code.
  • Auto-formatted whitespace differences — CI enforces formatting.
  • 生成文件的内容作为代码质量问题——它们是机械生成的产物。（但如果CRD变更的PR中缺少生成文件，仍属于有效结论。）
  • § 注意事项中声明为有意为之的内容。
  • 内部函数缺少godoc——这不是项目约定。
  • Dependabot的Go模块版本升级——属于可信自动化操作。
  • 未匹配代码的提前
    docs/
    或
    examples/
    更新。
  • 自动格式化导致的空白差异——CI会强制执行格式规范。

Delivery

交付方式

By default, reply the review back to the user in your response — do not post it to GitHub. Post it (e.g. 
gh pr comment
, using the access method from the GitHub credentials section) only when the user explicitly asks you to.
默认情况下，请将评审结果回复给用户——不要发布到GitHub。仅当用户明确要求时，才发布到GitHub（例如使用GitHub凭证章节中的访问方式执行
gh pr comment
）。

Output format

输出格式

Render the review as GitHub-flavored Markdown — clean enough to drop straight into a PR comment. Use the emoji legend below so the verdict is scannable at a glance. For any dimension that is PASS, write a single one-line summary naming the area at a high level — the ✅ in the section heading already carries the verdict, so do not repeat 
✅ PASS
in the body. No 
file:line
lists, no recap of what passed. Reserve justification, references and failure scenarios for CONCERN / FAIL.
Emoji legend:
  • Dimension verdict: ✅ PASS · ⚠️ CONCERN · ❌ FAIL
  • Overall assessment: ✅ Approve · 📝 Approve with notes · 🔧 Request changes · 🚫 Block
  • Risk level: 🟢 LOW · 🟡 MEDIUM · 🔴 HIGH
Use this exact structure:
markdown
undefined
将评审结果渲染为GitHub风格的Markdown——格式需简洁，可直接粘贴到PR评论中。使用下方的表情图例，以便快速浏览结论。对于任何通过的维度，只需写一句总结性的话，提及领域层面的内容——章节标题中的✅已经代表了结论，因此无需在正文中重复
✅ PASS
。不要列出
文件:行号
，不要重述通过的内容。仅为关注/失败的维度提供理由、引用和失败场景。
表情图例：
  • 维度结论：✅ 通过 · ⚠️ 关注 · ❌ 失败
  • 整体评估：✅ 批准 · 📝 附说明批准 · 🔧 要求修改 · 🚫 阻止
  • 风险等级：🟢 低 · 🟡 中 · 🔴 高
请使用以下精确结构：
markdown
undefined

🔍 PR Review

🔍 PR 评审

<1-2 sentences: what the PR does and why>
<1-2句话：PR的功能和目的>

Verdict at a glance

结论概览

DimensionResult
Correctness✅ PASS
Safety✅ PASS
Pitfall Detection⚠️ CONCERN
Backwards Compatibility✅ PASS
Code Quality✅ PASS
Overall: 📝 Approve with notes — Risk: 🟡 MEDIUM
维度结果
正确性✅ 通过
安全性✅ 通过
潜在陷阱检测⚠️ 关注
向后兼容性✅ 通过
代码质量✅ 通过
整体结论： 📝 附说明批准 — 风险等级： 🟡 中

✅ Correctness

✅ 正确性

<verdict; on CONCERN/FAIL add justification with 
file:line
references and failure scenario>
<结论；如果是关注/失败，请添加带有
文件:行号
引用和失败场景的理由>

✅ Safety

✅ 安全性

<verdict; on CONCERN/FAIL add justification with 
file:line
references and failure scenario>
<结论；如果是关注/失败，请添加带有
文件:行号
引用和失败场景的理由>

⚠️ Pitfall Detection

⚠️ 潜在陷阱检测

<verdict; on CONCERN/FAIL add justification with 
file:line
references and failure scenario>
<结论；如果是关注/失败，请添加带有
文件:行号
引用和失败场景的理由>

✅ Backwards Compatibility

✅ 向后兼容性

<verdict; on CONCERN/FAIL bullets classifying each finding as additive / behavioral / breaking>
<结论；如果是关注/失败，请用项目符号将每个结论分类为新增 / 行为变更 / 破坏性>

✅ Code Quality

✅ 代码质量

<verdict; on CONCERN/FAIL add justification with 
file:line
references>
<结论；如果是关注/失败，请添加带有
文件:行号
引用的理由>

📋 Suggested before merge

📋 合并前建议

<actionable items as a checklist, or "None">

Match the emoji in each dimension heading and in the summary table to that dimension's actual verdict, and set
the **Overall** and **Risk** emoji accordingly — the table and the section headings must agree.
<可操作的检查项列表，或“无”>

请确保每个维度标题和总结表格中的表情与该维度的实际结论一致，并相应设置**整体结论**和**风险等级**的表情——表格和章节标题必须保持一致。