Skip to content

*: use general approvers for BUILD.bazel files - #70391

Merged
ti-chi-bot[bot] merged 1 commit into
pingcap:masterfrom
D3Hunter:codex/general-build-bazel-approval
Aug 10, 2026
Merged

ti-chi-bot[bot] merged 1 commit into
pingcap:masterfrom
D3Hunter:codex/general-build-bazel-approval

Conversation

@D3Hunter

@D3Hunter D3Hunter commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: close #70390

Problem Summary:

make bazel_prepare can update BUILD.bazel files in packages whose source code was not changed by a pull request. Since Prow uses the nearest matching OWNERS file, those generated updates can require unrelated package-specific approvals.

What changed and how does it work?

Add an exact-basename BUILD.bazel filter to all 43 nested OWNERS files and assign sig-community-approvers to it. Prow unions approvers from matching filters, so package owners remain eligible while a general repository approver can approve generated Bazel files across packages.

For simple ownership configurations, move the existing approvers and labels unchanged under an .* filter. Existing options, special filters, labels, reviewers, and non-Bazel approval behavior are preserved.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No need to test
    • I checked and no code files have been changed.

Manual validation:

  • Parsed every nested OWNERS file as YAML and compared its effective structure with the parent commit. The check verified that only the exact BUILD.bazel filter was added and all prior rules were preserved.
  • Verified that the regular expression matches BUILD.bazel and nested a/BUILD.bazel, but not NOTBUILD.bazel or BUILD.bazel.tmp.
  • Ran git diff --check and confirmed the change is limited to 43 nested OWNERS files.

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

Summary by CodeRabbit

  • Chores
    • Updated code ownership rules across multiple components to use path-specific approval requirements.
    • BUILD.bazel changes now require community approval.
    • Other files retain their existing component-specific approvers, labels, and ownership settings.

@D3Hunter D3Hunter added type/enhancement The issue or PR belongs to an enhancement. component/build labels Aug 7, 2026
@ti-chi-bot ti-chi-bot Bot added release-note-none Denotes a PR that doesn't merit a release note. component/dumpling This is related to Dumpling of TiDB. component/statistics size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. sig/planner SIG: Planner labels Aug 7, 2026
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7b7de3fa-d336-4c18-a113-2b6ac7ce8946

📥 Commits

Reviewing files that changed from the base of the PR and between bd9159e and efd25be.

📒 Files selected for processing (43)
  • br/OWNERS
  • dumpling/OWNERS
  • lightning/OWNERS
  • lightning/cmd/tidb-lightning-ctl/OWNERS
  • lightning/cmd/tidb-lightning/OWNERS
  • pkg/autoid_service/OWNERS
  • pkg/bindinfo/OWNERS
  • pkg/config/OWNERS
  • pkg/ddl/OWNERS
  • pkg/distsql/OWNERS
  • pkg/domain/OWNERS
  • pkg/dumpformat/OWNERS
  • pkg/dxf/OWNERS
  • pkg/executor/OWNERS
  • pkg/executor/aggfuncs/OWNERS
  • pkg/executor/aggregate/OWNERS
  • pkg/executor/importer/OWNERS
  • pkg/executor/join/OWNERS
  • pkg/executor/mppcoordmanager/OWNERS
  • pkg/executor/sortexec/OWNERS
  • pkg/executor/test/analyzetest/OWNERS
  • pkg/executor/unionexec/OWNERS
  • pkg/expression/OWNERS
  • pkg/infoschema/OWNERS
  • pkg/ingestor/OWNERS
  • pkg/lightning/OWNERS
  • pkg/lightning/config/OWNERS
  • pkg/lock/OWNERS
  • pkg/meta/OWNERS
  • pkg/meta/metadef/OWNERS
  • pkg/metrics/OWNERS
  • pkg/objstore/OWNERS
  • pkg/owner/OWNERS
  • pkg/parser/OWNERS
  • pkg/planner/OWNERS
  • pkg/resourcemanager/OWNERS
  • pkg/session/OWNERS
  • pkg/sessionctx/vardef/OWNERS
  • pkg/sessionctx/variable/OWNERS
  • pkg/statistics/OWNERS
  • pkg/table/OWNERS
  • pkg/tablecodec/OWNERS
  • pkg/util/naming/OWNERS

📝 Walkthrough

Walkthrough

This change updates nested OWNERS files to route exact BUILD.bazel changes to sig-community-approvers. Existing approvers, labels, and other path rules remain unchanged for non-Bazel files.

Changes

OWNERS filter rollout

Layer / File(s) Summary
Split existing approval rules
br/OWNERS, dumpling/OWNERS, lightning/cmd/.../OWNERS, pkg/autoid_service/OWNERS, pkg/bindinfo/OWNERS, pkg/ddl/OWNERS, pkg/distsql/OWNERS, pkg/domain/OWNERS, pkg/dumpformat/OWNERS, pkg/dxf/OWNERS, pkg/executor/*/OWNERS, pkg/expression/OWNERS, pkg/infoschema/OWNERS, pkg/ingestor/OWNERS, pkg/lightning/*/OWNERS, pkg/lock/OWNERS, pkg/meta/OWNERS, pkg/metrics/OWNERS, pkg/objstore/OWNERS, pkg/owner/OWNERS, pkg/planner/OWNERS, pkg/resourcemanager/OWNERS, pkg/statistics/OWNERS, pkg/table/OWNERS, pkg/tablecodec/OWNERS, pkg/util/naming/OWNERS
Global approval rules now distinguish BUILD.bazel files from all other paths. Existing package approvers and labels remain assigned to non-Bazel paths.
Add BUILD.bazel approval filters
lightning/OWNERS, pkg/config/OWNERS, pkg/executor/OWNERS, pkg/meta/metadef/OWNERS, pkg/parser/OWNERS, pkg/session/OWNERS, pkg/sessionctx/vardef/OWNERS, pkg/sessionctx/variable/OWNERS
Each OWNERS file adds a BUILD.bazel filter with sig-community-approvers.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

A rabbit checks each BUILD file,
With community approval in style.
Other owners stay in their lane,
Labels and rules remain the same.
Hop, hop—the filters are done!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: allowing general approvers for BUILD.bazel files.
Description check ✅ Passed The description includes the issue, problem, implementation, validation steps, side effects, documentation, and release note sections.
Linked Issues check ✅ Passed The changes add the required BUILD.bazel filters, preserve existing ownership rules, and support general approval for generated Bazel changes.
Out of Scope Changes check ✅ Passed All changes are limited to the 43 nested OWNERS files and directly support the linked issue objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.6662%. Comparing base (bd9159e) to head (efd25be).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@               Coverage Diff                @@
##             master     #70391        +/-   ##
================================================
+ Coverage   76.3321%   76.6662%   +0.3341%     
================================================
  Files          2041       2053        +12     
  Lines        558851     566103      +7252     
================================================
+ Hits         426583     434010      +7427     
+ Misses       131368     130246      -1122     
- Partials        900       1847       +947     
Flag Coverage Δ
integration 49.4187% <ø> (+9.7500%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
dumpling 59.8974% <ø> (ø)
parser ∅ <ø> (∅)
br 63.6222% <ø> (+0.9131%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@goransh-walia

Copy link
Copy Markdown

This looks like a solid, focused change.

@ti-chi-bot ti-chi-bot Bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Aug 10, 2026
@ti-chi-bot

ti-chi-bot Bot commented Aug 10, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: djshow832, qw4990, wuhuizuo
Once this PR has been reviewed and has the lgtm label, please assign bb7133, benjamin2037, gengliqi, terry1purcell, v01dstar, zimulala for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot

ti-chi-bot Bot commented Aug 10, 2026

Copy link
Copy Markdown

@wuhuizuo: adding LGTM is restricted to approvers and reviewers in OWNERS files.

Details

In response to this:

Instructions 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 ti-chi-bot Bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Aug 10, 2026
@ti-chi-bot

ti-chi-bot Bot commented Aug 10, 2026

Copy link
Copy Markdown

[LGTM Timeline notifier]

Timeline:

  • 2026-08-10 07:53:14.107855216 +0000 UTC m=+3032980.143950262: ☑️ agreed by qw4990.
  • 2026-08-10 07:53:39.737018037 +0000 UTC m=+3033005.773113123: ☑️ agreed by djshow832.

@ti-chi-bot
ti-chi-bot Bot merged commit 94abd9f into pingcap:master Aug 10, 2026
36 of 60 checks passed
@D3Hunter
D3Hunter deleted the codex/general-build-bazel-approval branch August 10, 2026 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved component/build component/dumpling This is related to Dumpling of TiDB. component/statistics lgtm release-note-none Denotes a PR that doesn't merit a release note. sig/planner SIG: Planner size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. type/enhancement The issue or PR belongs to an enhancement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

build: avoid package-specific approval for generated BUILD.bazel changes

6 participants