OWNERS: sync master to release-8.5 - #69082
ti-chi-bot wants to merge 0 commit into
Conversation
|
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. |
📝 WalkthroughWalkthroughThis PR restructures the TiDB repository's approval governance by converting the root OWNERS file from static membership lists to filter-based path routing, extensively updating OWNERS_ALIASES with new team aliases and reorganized rosters, and establishing granular OWNERS configurations across 22 pkg/ subdirectories to enable specialized team approvals for executor, statistics, session, lightning, and domain-specific code paths. ChangesOwnership governance restructure
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/metrics/OWNERS (1)
2-5: ⚡ Quick winConsider adding labels for consistency with other OWNERS files.
Other OWNERS files in the repository include a
labelssection (e.g.,pkg/planner/OWNERShassig/planner,pkg/statistics/OWNERShas bothsig/plannerandcomponent/statistics). Adding labels likesig/metricsorcomponent/metricswould improve PR routing and maintain consistency with the governance structure.📝 Suggested addition
approvers: - sig-approvers-metrics +labels: + - component/metrics🤖 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/metrics/OWNERS` around lines 2 - 5, Add a labels section to this OWNERS file to match repository conventions: include at least "sig/metrics" and optionally "component/metrics" under a new labels key so PR routing and governance are consistent; update the block that currently contains "options:" and "approvers:" (referencing the existing options and approvers entries) to add the labels list.
🤖 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 3-11: The fallback filter '.*' is overlapping with the 'go\.mod$'
filter causing approvers/reviewers to be unioned; update the fallback key (the
one currently '.*') to a negative-lookahead regex that excludes 'go\.mod$' (for
example use a pattern like '^(?!go\.mod$).*') so files matching 'go\.mod$' only
get the 'go\.mod$' approvers and reviewers and not the fallback ones.
---
Nitpick comments:
In `@pkg/metrics/OWNERS`:
- Around line 2-5: Add a labels section to this OWNERS file to match repository
conventions: include at least "sig/metrics" and optionally "component/metrics"
under a new labels key so PR routing and governance are consistent; update the
block that currently contains "options:" and "approvers:" (referencing the
existing options and approvers entries) to add the labels list.
🪄 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: 93c730d4-5751-4cd5-a276-8d8a8a894a0c
📒 Files selected for processing (21)
OWNERSOWNERS_ALIASESpkg/dumpformat/OWNERSpkg/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
| filters: | ||
| 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 path matches both a specific filter like go.mod$and a fallback filter like.*, are approversandreviewers merged from both filters or does one filter override the other?
💡 Result:
If a single OWNERS file has both a specific filter (e.g. go\.mod$) and a fallback filter (e.g. .*) whose regexes both match the same relative path, Prow merges the approvers and reviewers from all matching filters rather than having one override the other. In Kubernetes’ OWNERS filters format, each regex key maps to a “simple OWNERS configuration,” and path regexes are matched against paths relative to the OWNERS file where they are declared [1]. The Prow implementation represents approvers/reviewers as “layered” sets over the applicable OWNERS configuration, and the core question of whether entries from multiple layers accumulate is handled by set operations (union) when building the final approver candidates: OWNERS file–scoped approvers are unioned together [2]. Additionally, the repoowners logic computes approvers/reviewers “for the requested file” using layered entries across matching OWNERS content (including parent dirs), rather than choosing a single matching rule [3]. So, for your example where both go\.mod$ and .* match the same path under the same OWNERS file, the resulting approver/reviewer sets are the union of both filters’ approvers/reviewers entries (with deduplication because they’re sets) [3][2].
Citations:
- 1: https://github.com/kubernetes/community/blob/main/contributors/guide/owners.md
- 2: https://github.com/kubernetes-sigs/prow/blob/main/pkg/plugins/approve/approvers/owners.go
- 3: https://github.com/kubernetes-sigs/prow/blob/master/pkg/repoowners/repoowners.go
Fix filter overlap: go.mod also matches fallback .*, so approvers/reviewers get unioned
In Prow Kubernetes OWNERS filters, entries from all matching regex keys accumulate (union) rather than one overriding the other. With go.mod, both go\.mod$ and .* match, so sig-community-approvers/sig-community-reviewers are added in addition to sig-critical-approvers-dep (OWNERS lines 3-11). Adjust the fallback .* to exclude go\.mod to preserve the dependency-critical approval boundary.
🤖 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 3 - 11, The fallback filter '.*' is overlapping with the
'go\.mod$' filter causing approvers/reviewers to be unioned; update the fallback
key (the one currently '.*') to a negative-lookahead regex that excludes
'go\.mod$' (for example use a pattern like '^(?!go\.mod$).*') so files matching
'go\.mod$' only get the 'go\.mod$' approvers and reviewers and not the fallback
ones.
d9c9e98 to
df2282c
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 |
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