chore: make helm versioning follow repo version - #696
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesHelm release versioning
Sequence Diagram(s)sequenceDiagram
participant ReleaseOperator
participant PlanRelease
participant StageHelm
participant GitRepository
ReleaseOperator->>PlanRelease: submit helm-version and release inputs
PlanRelease->>PlanRelease: validate stable Helm-only SemVer override
PlanRelease-->>StageHelm: provide resolved helm_version
StageHelm->>GitRepository: resolve nightly base version from merged tags
GitRepository-->>StageHelm: return release or RC tag
StageHelm-->>ReleaseOperator: package chart with chart_version and selected registry
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/scripts/resolve_helm_chart_version.py (1)
86-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test coverage for the CLI entrypoint (
main/parse_args).The "Resolver CLI contract" layer (argument parsing, stdout printing,
HelmVersionError→ stderr + exit code 1) has no tests; only the internalchart_version/latest_stable_release_core/resolve_helm_chart_versionfunctions are tested. Consider adding a test invokingmain()for both the success path (stdout version, exit 0) and failure path (stderr message, exit 1).🤖 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 @.github/scripts/resolve_helm_chart_version.py around lines 86 - 116, Add CLI-level tests for parse_args and main, covering successful resolution with the version printed to stdout and exit code 0, plus HelmVersionError handling with the message printed to stderr and exit code 1. Mock argument parsing and resolve_helm_chart_version as needed, while preserving the existing internal resolver tests.
🤖 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.
Nitpick comments:
In @.github/scripts/resolve_helm_chart_version.py:
- Around line 86-116: Add CLI-level tests for parse_args and main, covering
successful resolution with the version printed to stdout and exit code 0, plus
HelmVersionError handling with the message printed to stderr and exit code 1.
Mock argument parsing and resolve_helm_chart_version as needed, while preserving
the existing internal resolver tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7d0e9bb7-4034-41d7-9a09-6098637e60b2
📒 Files selected for processing (3)
.github/scripts/resolve_helm_chart_version.py.github/scripts/tests/test_resolve_helm_chart_version.py.github/workflows/release.yaml
Contributor
|
mckornfield
force-pushed
the
helm-versioning/mck
branch
from
July 15, 2026 14:52
d2b753e to
5ee71cf
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.github/scripts/resolve_helm_chart_version.py:
- Around line 49-64: Update latest_stable_release_core() to surface git command
failures instead of returning None when result.returncode is nonzero. Emit the
captured result.stderr or raise HelmVersionError with the command failure
details, while preserving the existing None return for successful commands with
no matching stable tags.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0cd7403e-c4eb-464c-9b11-e26924dc9e92
📒 Files selected for processing (3)
.github/scripts/resolve_helm_chart_version.py.github/scripts/tests/test_resolve_helm_chart_version.py.github/workflows/release.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/scripts/tests/test_resolve_helm_chart_version.py
- .github/workflows/release.yaml
mckornfield
force-pushed
the
helm-versioning/mck
branch
from
July 15, 2026 17:54
5ee71cf to
51d3393
Compare
* Also allow for an override case Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
mckornfield
force-pushed
the
helm-versioning/mck
branch
from
July 15, 2026 17:54
51d3393 to
6db7ede
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
RELEASING.md (1)
59-93: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftSplit this into Diataxis pages.
Keep this page as a HOW-TO, move input semantics to REFERENCE and version-resolution behavior to EXPLANATION, then cross-link them. Put Helm override prerequisites first and add a
Next Stepssection.As per coding guidelines, “Each documentation page should fit ONE Diataxis quadrant,” “Always list prerequisites at the top of documentation pages,” and “Include 'Next Steps' section at the end.”
🤖 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 `@RELEASING.md` around lines 59 - 93, Refactor RELEASING.md into a HOW-TO by retaining only procedural release steps, moving input semantics into a linked REFERENCE page and version-resolution behavior into a linked EXPLANATION page. Place Helm override prerequisites at the top, add links to both new pages, and append a “Next Steps” section with relevant follow-up links.Source: Coding guidelines
🤖 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 @.github/workflows/release.yaml:
- Around line 290-291: Update the helmVersionOverride validation in the release
workflow to use strict SemVer parsing or a standards-compliant pattern,
rejecting leading-zero numeric components and malformed prerelease identifiers
such as empty dot-separated segments. Ensure invalid helm-version values fail
during planning while preserving valid SemVer support.
---
Outside diff comments:
In `@RELEASING.md`:
- Around line 59-93: Refactor RELEASING.md into a HOW-TO by retaining only
procedural release steps, moving input semantics into a linked REFERENCE page
and version-resolution behavior into a linked EXPLANATION page. Place Helm
override prerequisites at the top, add links to both new pages, and append a
“Next Steps” section with relevant follow-up links.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7d394c3a-9557-48af-ad3c-98d9e4ec09a3
📒 Files selected for processing (2)
.github/workflows/release.yamlRELEASING.md
Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
crookedstorm
approved these changes
Jul 15, 2026
crookedstorm
left a comment
crookedstorm
left a comment
Contributor
There was a problem hiding this comment.
Looks good. Follows the general patterns of the repo, and we can always try a manual nightly once all the containers are fixed.
This was referenced Jul 23, 2026
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
2 participants
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.
Summary by CodeRabbit
helm-versionoverride for stable, Helm-only releases, with validation.helm-versioninput, including an example and updated “What the workflow does” Helm versioning details.