Fix milestone fallback: inflight/candidate reads from main, not staging branch#35054
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35054Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35054" |
370316f to
63debc8
Compare
There was a problem hiding this comment.
Pull request overview
Adjusts milestone drift detection so that PRs targeting staging branches (inflight/*, darc/*) derive version info from origin/main (the eventual shipping target) rather than from potentially stale staging-branch Versions.props.
Changes:
- Update the fallback path in
Invoke-AnalyzeSinglePrto readVersions.propsfromorigin/mainforinflight/*anddarc/*base refs; otherwise read fromorigin/<baseRef>. - Fix
Get-VersionFromGitReffetching behavior to avoid attempting to fetchorigin/origin/<branch>when the input already includes anorigin/prefix. - Add a “live validation guide” comment block to the Pester test file (but it needs updates to match the new behavior).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
.github/scripts/Fix-MilestoneDrift.ps1 |
Implements the new fallback source-of-truth (main for staging branches) and corrects the git fetch refspec handling. |
.github/scripts/Fix-MilestoneDrift.Tests.ps1 |
Adds documentation for manual validation steps; currently contains expectations that conflict with the updated staging-branch behavior. |
| # 1. PR merged to inflight/current — should read Versions.props from origin/inflight/current | ||
| pwsh -File .github/scripts/Fix-MilestoneDrift.ps1 -PrNumber 34228 -RepoPath . -Verbose | ||
| # Expected: reads from origin/inflight/current, milestone matches current PatchVersion on that branch | ||
|
|
There was a problem hiding this comment.
The live validation guide is now out of date: the script’s new fallback logic reads Versions.props from origin/main for staging branches (inflight/, darc/), but this example still says inflight/current should read from origin/inflight/current. Please update the guidance to reflect the new behavior and expected output.
| Key things to verify after changes: | ||
| - inflight/current PRs read from origin/inflight/current (not stale merge commit) | ||
| - net11.0 PRs read from origin/net11.0 (never from origin/main) | ||
| - PRs on release branches get the milestone from the branch name (not Versions.props) |
There was a problem hiding this comment.
These verification bullets don’t match the updated behavior: inflight/* and darc/* PRs should now read Versions.props from origin/main, not origin/inflight/current. Updating this list will prevent future changes from being validated against the old expectations.
| # the PR was merged to. | ||
| $mainBranch = Get-MainBranchForVersion $detectedMajor $Repo | ||
| $devBranch = "origin/$mainBranch" | ||
| $branchVersionInfo = Get-VersionFromGitRef $devBranch $Repo | ||
| if ($branchVersionInfo) { | ||
| $versionInfo = $branchVersionInfo | ||
| $ReleaseTag = $branchVersionInfo.Tag | ||
| $preLabel = $branchVersionInfo.PreLabel | ||
| $preIter = if ($branchVersionInfo.PreIter) { $branchVersionInfo.PreIter } else { 0 } | ||
| $preDisplay = if ($branchVersionInfo.PreLabel) { " ($($branchVersionInfo.PreLabel)$($branchVersionInfo.PreIter))" } else { "" } |
There was a problem hiding this comment.
The new branch-selection behavior for the fallback path (choosing origin/main for inflight/* and darc/; otherwise origin/) isn’t covered by the existing Pester tests. Consider extracting the dev-branch selection into a small pure helper (e.g., taking BaseRef and returning the git ref) and add unit tests for inflight/, darc/, main, net11.0, and release/ cases using Pester without requiring live git/gh access.
487e8d8 to
1543ba1
Compare
PR #35054 — Multi-Model Code Review (Final)CI Status: Prior reviews: Copilot reviewer comments addressed (stale docs). Previous round: 2/3 clean, 1 timed out. Review Results
Adversarial ConsensusReviewer 3's finding — REJECTED (1/3, disputed):
For a PR merged directly to Summary
Recommended Action✅ Approve — Zero confirmed issues. All validation passes. Diff is clean (47 insertions, 32 deletions). |
Two fixes to milestone detection:
1. Fallback reads Versions.props from the development branch (main
for .NET 10, net11.0 for .NET 11) instead of origin/{base.ref}.
Staging branches like inflight/candidate have stale PatchVersion
but their PRs ultimately ship on main's version.
2. Find-ReleaseBranchForCommit falls back to git log --grep when
ancestry check fails. Handles rebases and cherry-picks where the
commit SHA changes but the PR number '(#NNNNN)' is preserved in
the commit message.
Updated live validation guide to match new behavior.
Validated:
- #34667 (rebased to SR6) → found via grep → .NET 10 SR6 ✅
- #34959 (inflight/candidate) → reads main → .NET 10 SR7 ✅
- #34620 (on SR6 branch) → ancestry → .NET 10 SR6 ✅
- #34969 (net11.0) → reads net11.0 → .NET 11.0-preview4 ✅
91 Pester tests pass.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1543ba1 to
5f09344
Compare
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Summary
Fix milestone fallback for
inflight/*anddarc/*branches to read fromorigin/maininstead of the staging branch itself.Problem
PRs merged to
inflight/candidatewere milestoned as SR6 becauseinflight/candidatehasPatchVersion=60. But those PRs will ultimately ship in SR7 (when the Candidate merges tomain, which is atPatchVersion=70).Fix
For staging branches (
inflight/*,darc/*), readVersions.propsfromorigin/maininstead oforigin/{base.ref}. These branches always feed into main, so main's version is the correct target.All other branches continue reading from
origin/{base.ref}directly.Validated
91 Pester tests pass.