fix(release): keep version bump branches linear - #757
Conversation
📝 WalkthroughWalkthroughThe release workflow now creates version-bump branches from a linearized release base. Tests verify branch ancestry, version-only changes, and the absence of merge commits. The NVCA version changes to ChangesRelease branch linearity
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Start generated VERSION bump branches from the same linear snapshot used for release branches when the selected history contains merges. Advance NVCA to 3.3.0 for the missing release-train bump. Refs: #756 Signed-off-by: Kristina Pathak <kpathak@nvidia.com>
b50e00c to
494c86d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tools/ci/test-github-release.py (2)
637-642: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftExercise
branch_cutin this regression.This block manually creates the bump branch and commits
VERSION. It does not execute the changedbranch_cutlines 924-925. A regression in the production wiring can therefore pass. Add a publish-mode integration case with a temporary remote, stub thegh pr createcall, and inspect the pushed bump branch.As per coding guidelines, changed tool behavior requires focused tests.
🤖 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 `@tools/ci/test-github-release.py` around lines 637 - 642, Add a publish-mode integration test alongside the existing release regression setup that invokes the production branch_cut flow with a temporary remote, stubs gh pr create, and verifies the pushed release-bump branch and VERSION commit. Replace the manual branch creation and commit in this regression with the branch_cut invocation so the changed production wiring is exercised.Source: Coding guidelines
650-655: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the pull-request diff, not only the tree diff.
Line 652 compares the two final trees directly. It does not check the three-dot diff used by a pull request. With this merge fixture, the PR-style diff also contains
merged.txtandsrc/compute-plane-services/nvca/README.md. Add a separate merge-base diff assertion and keep the current comparison as the tree-preservation check.🤖 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 `@tools/ci/test-github-release.py` around lines 650 - 655, Add a separate assertion in the test around the existing git diff check that compares the PR-style three-dot diff from the merge base to bump_head, expecting merged.txt and src/compute-plane-services/nvca/README.md. Preserve the current two-commit tree diff assertion unchanged as the tree-preservation check, using the existing self.github_release.run invocation.
🤖 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 `@tools/ci/github-release`:
- Around line 924-925: Update the release branch setup around
linear_release_branch_base and the git switch so pull-request comparison
preserves the selected base commit, either by branching from that commit or
comparing against the linearized branch. In tools/ci/test-github-release.py
lines 650-655, add a three-dot merge-base diff assertion while retaining the
existing direct tree comparison separately.
---
Nitpick comments:
In `@tools/ci/test-github-release.py`:
- Around line 637-642: Add a publish-mode integration test alongside the
existing release regression setup that invokes the production branch_cut flow
with a temporary remote, stubs gh pr create, and verifies the pushed
release-bump branch and VERSION commit. Replace the manual branch creation and
commit in this regression with the branch_cut invocation so the changed
production wiring is exercised.
- Around line 650-655: Add a separate assertion in the test around the existing
git diff check that compares the PR-style three-dot diff from the merge base to
bump_head, expecting merged.txt and src/compute-plane-services/nvca/README.md.
Preserve the current two-commit tree diff assertion unchanged as the
tree-preservation check, using the existing self.github_release.run invocation.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 99f861d7-b628-4ba7-ade8-3be76846ca20
📒 Files selected for processing (2)
tools/ci/github-releasetools/ci/test-github-release.py
| bump_base = linear_release_branch_base(root, base_sha) | ||
| run(["git", "switch", "-C", bump_branch, bump_base], cwd=root) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
The linearized snapshot changes the pull-request merge base.
When the selected base contains merges, the synthetic snapshot is rooted before the first merge. The resulting pull request can include unrelated files even when the final trees differ only in VERSION.
tools/ci/github-release#L924-L925: use a branch strategy that preserves the selected commit for the pull-request comparison, or compare the pull request against the linearized branch.tools/ci/test-github-release.py#L650-L655: add a three-dot merge-base diff assertion and retain the direct tree comparison separately.
📍 Affects 2 files
tools/ci/github-release#L924-L925(this comment)tools/ci/test-github-release.py#L650-L655
🤖 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 `@tools/ci/github-release` around lines 924 - 925, Update the release branch
setup around linear_release_branch_base and the git switch so pull-request
comparison preserves the selected base commit, either by branching from that
commit or comparing against the linearized branch. In
tools/ci/test-github-release.py lines 650-655, add a three-dot merge-base diff
assertion while retaining the existing direct tree comparison separately.
1 participant
TL;DR
Additional Details
For the Reviewer
For QA
python3 tools/ci/test-github-release.pypython3 tools/ci/github-release branch-cut --service nvca --dry-runpython3 -m py_compile tools/ci/github-release tools/ci/test-github-release.pyIssues
Fixes #756
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Tests