chore: request code owners only on pull requests that reach next - #25455
Draft
AztecBot wants to merge 2 commits into
Draft
chore: request code owners only on pull requests that reach next#25455AztecBot wants to merge 2 commits into
AztecBot wants to merge 2 commits into
Conversation
…her, log why nothing was requested
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Since #25435 set
CODEOWNERSfor the Foundation-owned components, owners are beingrequested on pull requests that are not headed for
next— stacked pull requestswhose base is another pull request's branch — and re-requested every time the stack
is rebased.
That is how GitHub's native
CODEOWNERSworks, and there is no setting that changes it:the file is read from the pull request's base branch, every feature branch cut from
nextcarries a copy, and owner assignment re-runs when the base moves. Observed on#25436: each
review_requestedevent lands in the same second as abase_ref_force_pushed, and the PR has had four furtherbase_ref_changedeventssince.
What this does
Requests owners only when a pull request's merge reaches
next:next, ornext— everymerge-train/*except the-v<N>-suffixed ones, which target their release line (the same rulemerge-train-create-pr.ymluses to open the train's own pull request).Stacked pull requests request nobody. Rebasing a stack requests nobody. When the pull
request under a stack merges and the next one is retargeted onto
next, owners arerequested once, at that moment.
How
CODEOWNERSis renamed to.github/next-code-owners, content unchanged apart from anexplanatory header (Git records it as an 80%-similar rename). GitHub reads only the
exact name
CODEOWNERS, so once renamed it requests nobody natively, on any base..github/workflows/codeowner-notify-next.ymlruns onpull_request_targetforopened,ready_for_review, andedited(the last only when the edit changed thebase), prefiltered to
nextandmerge-train/**so ordinary pull requests start nojob. Drafts are skipped. It checks out only the two files it needs, from the base
branch — never the pull request's head — so the elevated token is not exposed to code
from the pull request.
.github/scripts/next_code_owners.pyapplies the file withCODEOWNERSsemantics:last matching pattern wins per changed path, reviewers are the union across paths,
@org/teambecomes a team reviewer, and the author is dropped (GitHub rejectsrequesting the author).
Preserving last-match-wins matters here: the file relies on it and says so in its own
comments —
/barretenberg/sits above the AVM block so the AVM team re-claimsvm2,and the protocol-contracts rule precedes the
public_context.nrline for the samereason. A union-of-all-matches matcher would ping the bb team for AVM changes.
Tests
22 cases against the real file, all passing:
vm2/foo.cpp→ AVM team only;public_context.nr→ AVM, not protocol;private-kernel-init/→ kernel owners (no IlyasRidhuan);rollup-base/→ rollupowners;
crates/types/→ the broad circuits rule; two files → union of both winners.mock-protocol-circuits,yarn-project/,avm-transpiler/Cargo.lock(whileCargo.tomlis owned).next,merge-train/barretenberg,merge-train/docsnotify;merge-train/spartan-v5,v5-next,cb/blob-fracs-halving(the stacked base fromtest(blobs): pin the fracs hint's root-of-unity contract and widen its coverage #25450),
ad/featuredo not.has not pulled
nextyet) requests nobody rather than failing.What is given up
GitHub's native code-owner features stop applying on
next: the "code owner" badge onreview requests, owner hints in the Files-changed view, and the
require_code_owner_reviewruleset option, which #25435 named as a possible next step.Reversing is a rename back.
Rollout
pull_request_targetruns the workflow from the base branch, so this takes effect onnextat merge and on each train once it has pullednext. Trains outside the auto-pullloop (
docs,fairies,spartan,test-train) keep the old nativeCODEOWNERS— andits native behaviour — until someone merges
nextinto them.v5-nextis untouched.The private repo receives this through the public → private sync; the two files are
byte-identical today.
Created by claudebox · group:
slackbot· requested by Mike (@iAmMichaelConnor) · Slack thread