OWNERS: sync master to release-8.5 - #68873
ti-chi-bot wants to merge 0 commit into
Conversation
ti-chi-bot
added
approved
cherry-pick-approved
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
ti-chi-bot
Bot
added
do-not-merge/release-note-label-needed
📝 WalkthroughWalkthroughThis PR refactors TiDB's code approval governance by converting the root OWNERS file to filter-based routing rules and substantially expanding SIG membership lists in OWNERS_ALIASES. It then propagates ownership configuration across package hierarchies through new OWNERS files in executor subdirectories, critical system packages, and component-specific modules, each designating appropriate SIG approver groups. ChangesOwnership Infrastructure Restructuring
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@OWNERS`:
- Around line 4-11: The go\.mod$ and .* filters overlap so both match go.mod and
aggregate approvers; update the OWNERS filters to be mutually exclusive by
narrowing the catch-all rule (the .* entry) so it does not match go.mod or by
replacing it with a non-overlapping fallback (e.g., exclude go\.mod via a
negative lookahead or list explicit file patterns), ensuring go\.mod$ only has
sig-critical-approvers-dep and the catch-all only applies to other files.
In `@pkg/session/OWNERS`:
- Around line 3-10: The OWNERS config currently sets no_parent_owners: true and
only two narrow filters, leaving other files without approvers; add a catch-all
rule so every file has reviewers—either add a top-level approvers list or append
a filter like ".*" (or "(^|/).*") with an appropriate approver group (e.g.,
sig-critical-approvers-tidb-server or another team) so any file not matched by
the existing patterns gets an approver; update the filters block (or add
top-level approvers) and ensure the new rule sits after the specific filters to
act as the default.
In `@pkg/sessionctx/vardef/OWNERS`:
- Around line 3-7: The OWNERS file only assigns approvers for the regex
"(OWNERS|(sysvar|tidb_vars)\\.go)$" and with no_parent_owners: true all other
files in pkg/sessionctx/vardef are left unowned; add a catch‑all filter (for
example a key with regex "^.*$" or similar) under filters and assign the
appropriate approvers (e.g. - sig-critical-approvers-tidb-server) so files not
matched by "(OWNERS|(sysvar|tidb_vars)\\.go)$" have fallback approvers; keep the
existing specific filter and no_parent_owners setting intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cd6b45a7-5620-48be-9dd3-749044cb75f1
📒 Files selected for processing (20)
OWNERSOWNERS_ALIASESpkg/dxf/OWNERSpkg/executor/OWNERSpkg/executor/aggfuncs/OWNERSpkg/executor/aggregate/OWNERSpkg/executor/join/OWNERSpkg/executor/mppcoordmanager/OWNERSpkg/executor/sortexec/OWNERSpkg/executor/test/analyzetest/OWNERSpkg/executor/unionexec/OWNERSpkg/ingestor/OWNERSpkg/lightning/config/OWNERSpkg/meta/metadef/OWNERSpkg/metrics/OWNERSpkg/objstore/OWNERSpkg/session/OWNERSpkg/sessionctx/vardef/OWNERSpkg/statistics/OWNERSpkg/util/naming/OWNERS
| go\.mod$: | ||
| approvers: | ||
| - sig-critical-approvers-dep | ||
| .*: | ||
| approvers: | ||
| - sig-community-approvers | ||
| reviewers: | ||
| - sig-community-reviewers |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Kubernetes/Prow OWNERS files, when a file matches both go.mod$and.*underfilters, are approvers/reviewers combined from both matching filters or is one filter selected?
💡 Result:
In Kubernetes/Prow OWNERS files, when multiple filters match a file path (such as both go\.mod$ and .*), the approvers and reviewers are combined from all matching filters [1]. Prow's repoowners logic processes filters by identifying all regular expressions that match the given file path [1]. Rather than selecting only the "best" or first match, the system aggregates the ownership configurations—including approvers and reviewers—from every matching entry [1]. This ensures that if a file is covered by both a specific rule (e.g., matching a file extension) and a catch-all rule (e.g., .*), the union of all listed owners is applied to that file [1].
Citations:
Major: Overlapping OWNERS filters cause go.mod to include community approvers/reviewers too
In OWNERS (lines 4-11), go\.mod$ and the catch-all .* both match go.mod; Prow’s repoowners aggregates approvers/reviewers from all matching filters, so sig-community-approvers/sig-community-reviewers will be added in addition to sig-critical-approvers-dep (https://github.com/kubernetes-sigs/prow/blob/master/pkg/repoowners/repoowners.go). Make the filters mutually exclusive (e.g., narrow the .* pattern so it does not match go.mod, or replace it with a non-overlapping fallback rule).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@OWNERS` around lines 4 - 11, The go\.mod$ and .* filters overlap so both
match go.mod and aggregate approvers; update the OWNERS filters to be mutually
exclusive by narrowing the catch-all rule (the .* entry) so it does not match
go.mod or by replacing it with a non-overlapping fallback (e.g., exclude go\.mod
via a negative lookahead or list explicit file patterns), ensuring go\.mod$ only
has sig-critical-approvers-dep and the catch-all only applies to other files.
| no_parent_owners: true | ||
| filters: | ||
| "(OWNERS|bootstrap\\.go)$": | ||
| "(OWNERS|upgrade_def\\.go)$": | ||
| approvers: | ||
| - sig-critical-approvers-tidb-server | ||
|
|
||
| "(bootstrap\\.go|upgrade_run\\.go)$": | ||
| approvers: | ||
| - sig-approvers-ddl |
There was a problem hiding this comment.
Add a default ownership rule to avoid uncovered files.
no_parent_owners: true plus only two narrow filters leaves non-matching files in pkg/session without local approvers. Add a catch-all filter (or top-level approvers) so every file remains reviewable.
Suggested fix
filters:
"(OWNERS|upgrade_def\\.go)$":
approvers:
- sig-critical-approvers-tidb-server
"(bootstrap\\.go|upgrade_run\\.go)$":
approvers:
- sig-approvers-ddl
+ ".*":
+ approvers:
+ - sig-approvers-ddl🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/session/OWNERS` around lines 3 - 10, The OWNERS config currently sets
no_parent_owners: true and only two narrow filters, leaving other files without
approvers; add a catch-all rule so every file has reviewers—either add a
top-level approvers list or append a filter like ".*" (or "(^|/).*") with an
appropriate approver group (e.g., sig-critical-approvers-tidb-server or another
team) so any file not matched by the existing patterns gets an approver; update
the filters block (or add top-level approvers) and ensure the new rule sits
after the specific filters to act as the default.
| no_parent_owners: true | ||
| filters: | ||
| "(OWNERS|(sysvar|tidb_vars)\\.go)$": | ||
| approvers: | ||
| - sig-critical-approvers-tidb-server |
There was a problem hiding this comment.
Define fallback approvers for files outside the critical-file regex.
With no_parent_owners: true, this file currently assigns approvers only for OWNERS, sysvar.go, and tidb_vars.go. Other files in pkg/sessionctx/vardef can become unowned.
Suggested fix
filters:
"(OWNERS|(sysvar|tidb_vars)\\.go)$":
approvers:
- sig-critical-approvers-tidb-server
+ ".*":
+ approvers:
+ - sig-approvers-meta📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| no_parent_owners: true | |
| filters: | |
| "(OWNERS|(sysvar|tidb_vars)\\.go)$": | |
| approvers: | |
| - sig-critical-approvers-tidb-server | |
| no_parent_owners: true | |
| filters: | |
| "(OWNERS|(sysvar|tidb_vars)\\.go)$": | |
| approvers: | |
| - sig-critical-approvers-tidb-server | |
| ".*": | |
| approvers: | |
| - sig-approvers-meta |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/sessionctx/vardef/OWNERS` around lines 3 - 7, The OWNERS file only
assigns approvers for the regex "(OWNERS|(sysvar|tidb_vars)\\.go)$" and with
no_parent_owners: true all other files in pkg/sessionctx/vardef are left
unowned; add a catch‑all filter (for example a key with regex "^.*$" or similar)
under filters and assign the appropriate approvers (e.g. -
sig-critical-approvers-tidb-server) so files not matched by
"(OWNERS|(sysvar|tidb_vars)\\.go)$" have fallback approvers; keep the existing
specific filter and no_parent_owners setting intact.
|
@ti-chi-bot: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
27f9ab3 to
0f820cb
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
ti-chi-bot
Bot
added
size/XS
1 participant
What problem does this PR solve?
Sync
OWNERS*files frommastertorelease-8.5.What changed and how does it work?
This PR was auto-generated by Prow and only updates
OWNERSandOWNERS_ALIASESfiles./release-note-none
skip-issue-check
lgtm
approved
cherry-pick-approved
Summary by CodeRabbit