Conversation
- Reset Chart.yaml version to 0.0.1 with explanatory comment - Update CHART_VERSION to use github.ref_name instead of Chart.yaml - Update helm-chart-oci-publisher to @17e4144d7ba68f7c3e8c16eece5aed15fd7c2dc8 🤖 Generated with [GitHub Copilot](https://github.com/features/copilot) (via Zed) Signed-off-by: Eric Searcy <eric@linuxfoundation.org>
WalkthroughThe workflow Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
There was a problem hiding this comment.
Pull request overview
This PR implements standardized chart release versioning by tying Helm chart versions to application release versions. Instead of manually maintaining the chart version in Chart.yaml, the version is now dynamically extracted from Git tags during the release workflow.
Changes:
- Reset Chart.yaml version to 0.0.1 as a placeholder with explanatory comments
- Modified workflow to derive CHART_VERSION from git tag instead of Chart.yaml
- Updated helm-chart-oci-publisher action to newer commit with bug fixes
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| charts/lfx-v2-mailing-list-service/Chart.yaml | Reset version to 0.0.1 placeholder with comments explaining dynamic replacement during build |
| .github/workflows/ko-build-tag.yaml | Changed CHART_VERSION extraction from Chart.yaml to git tag, updated helm-chart-oci-publisher commit, removed trailing spaces |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| APP_VERSION=$(echo ${{ github.ref_name }} | sed 's/v//g') | ||
| CHART_NAME="$(yq '.name' charts/*/Chart.yaml)" | ||
| CHART_VERSION="$(yq '.version' charts/*/Chart.yaml)" | ||
| CHART_VERSION=$(echo ${{ github.ref_name }} | sed 's/v//g') |
There was a problem hiding this comment.
The same version extraction logic is duplicated for both APP_VERSION and CHART_VERSION. Consider extracting this to a single variable to follow the DRY principle and reduce maintenance burden. For example:
VERSION=$(echo ${{ github.ref_name }} | sed 's/v//g')
APP_VERSION=$VERSION
CHART_VERSION=$VERSIONThis makes it clear that both versions should always be identical and reduces the chance of inconsistencies if the extraction logic needs to be modified in the future.
![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
🤖 Fix all issues with AI agents
In @.github/workflows/ko-build-tag.yaml:
- Around line 37-39: The APP_VERSION and CHART_VERSION assignments use sed
's/v//g' which removes all "v" characters and corrupts prerelease tags; update
the sed expression for both APP_VERSION and CHART_VERSION to only strip a
leading "v" (replace the global removal with a pattern that removes ^v) so tags
like v1.2.3-dev and v1.0-v2 keep their internal "v" characters intact.
| APP_VERSION=$(echo ${{ github.ref_name }} | sed 's/v//g') | ||
| CHART_NAME="$(yq '.name' charts/*/Chart.yaml)" | ||
| CHART_VERSION="$(yq '.version' charts/*/Chart.yaml)" | ||
| CHART_VERSION=$(echo ${{ github.ref_name }} | sed 's/v//g') |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Test sed behavior with various tag formats
echo "Testing sed 's/v//g' vs sed 's/^v//'"
echo "======================================="
for tag in "v1.2.3" "v1.2.3-dev" "v2.0.0-rc1" "v1.0-v2"; do
result_global=$(echo "$tag" | sed 's/v//g')
result_prefix=$(echo "$tag" | sed 's/^v//')
echo "Tag: $tag"
echo " sed 's/v//g': $result_global"
echo " sed 's/^v//': $result_prefix"
echo ""
doneRepository: linuxfoundation/lfx-v2-mailing-list-service
Length of output: 424
🏁 Script executed:
# Check if the workflow file exists and examine lines 37-39
if [ -f ".github/workflows/ko-build-tag.yaml" ]; then
echo "File found. Checking lines 37-39:"
sed -n '37,39p' ".github/workflows/ko-build-tag.yaml"
echo ""
echo "Extended context (lines 30-45):"
sed -n '30,45p' ".github/workflows/ko-build-tag.yaml"
else
echo "File not found"
fiRepository: linuxfoundation/lfx-v2-mailing-list-service
Length of output: 943
🏁 Script executed:
# Search for all uses of sed 's/v//g' or similar patterns in the workflow
echo "Searching for sed patterns in the workflow file:"
rg "sed.*'s/.*v" ".github/workflows/ko-build-tag.yaml" -A 1 -B 1Repository: linuxfoundation/lfx-v2-mailing-list-service
Length of output: 370
Strip only the leading "v" from version tags to avoid corruption of prerelease versions.
The current sed 's/v//g' removes all v characters globally. This corrupts prerelease tags: v1.2.3-dev becomes 1.2.3-de and v1.0-v2 becomes 1.0-2. Use sed 's/^v//' instead to strip only the leading v prefix.
Both APP_VERSION (line 37) and CHART_VERSION (line 39) have this issue:
Suggested fix
- APP_VERSION=$(echo ${{ github.ref_name }} | sed 's/v//g')
+ APP_VERSION=$(echo ${{ github.ref_name }} | sed 's/^v//')
CHART_NAME="$(yq '.name' charts/*/Chart.yaml)"
- CHART_VERSION=$(echo ${{ github.ref_name }} | sed 's/v//g')
+ CHART_VERSION=$(echo ${{ github.ref_name }} | sed 's/^v//')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| APP_VERSION=$(echo ${{ github.ref_name }} | sed 's/v//g') | |
| CHART_NAME="$(yq '.name' charts/*/Chart.yaml)" | |
| CHART_VERSION="$(yq '.version' charts/*/Chart.yaml)" | |
| CHART_VERSION=$(echo ${{ github.ref_name }} | sed 's/v//g') | |
| APP_VERSION=$(echo ${{ github.ref_name }} | sed 's/^v//') | |
| CHART_NAME="$(yq '.name' charts/*/Chart.yaml)" | |
| CHART_VERSION=$(echo ${{ github.ref_name }} | sed 's/^v//') |
🤖 Prompt for AI Agents
In @.github/workflows/ko-build-tag.yaml around lines 37 - 39, The APP_VERSION
and CHART_VERSION assignments use sed 's/v//g' which removes all "v" characters
and corrupts prerelease tags; update the sed expression for both APP_VERSION and
CHART_VERSION to only strip a leading "v" (replace the global removal with a
pattern that removes ^v) so tags like v1.2.3-dev and v1.0-v2 keep their internal
"v" characters intact.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/ko-build-tag.yaml (1)
37-39: Usesed 's/^v//'to avoid stripping internal “v” characters.Line 37 and Line 39 still use
sed 's/v//g', which removes all “v” characters and can corrupt prerelease tags. This is the same issue raised earlier.🔧 Suggested fix
- APP_VERSION=$(echo ${{ github.ref_name }} | sed 's/v//g') + APP_VERSION=$(echo ${{ github.ref_name }} | sed 's/^v//') CHART_NAME="$(yq '.name' charts/*/Chart.yaml)" - CHART_VERSION=$(echo ${{ github.ref_name }} | sed 's/v//g') + CHART_VERSION=$(echo ${{ github.ref_name }} | sed 's/^v//')#!/bin/bash # Verify tag transformations for tag in "v1.2.3" "v1.2.3-dev" "v1.0-v2"; do echo "Tag: $tag" echo " s/v//g -> $(echo "$tag" | sed 's/v//g')" echo " s/^v// -> $(echo "$tag" | sed 's/^v//')" done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ko-build-tag.yaml around lines 37 - 39, The current assignments for APP_VERSION and CHART_VERSION use sed 's/v//g', which strips every "v" in the tag; update both assignments to use sed 's/^v//' to only remove a leading "v" (preserving internal/prerelease "v" characters) — modify the lines that set APP_VERSION and CHART_VERSION accordingly while leaving CHART_NAME unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/ko-build-tag.yaml:
- Around line 37-39: The current assignments for APP_VERSION and CHART_VERSION
use sed 's/v//g', which strips every "v" in the tag; update both assignments to
use sed 's/^v//' to only remove a leading "v" (preserving internal/prerelease
"v" characters) — modify the lines that set APP_VERSION and CHART_VERSION
accordingly while leaving CHART_NAME unchanged.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/ko-build-tag.yaml
2 participants
Implements standardized chart release versioning for LFXV2-872.
Changes Applied
✅ Chart.yaml Updates:
0.1.9to0.0.1✅ Workflow Updates (
ko-build-tag.yaml):helm-chart-oci-publisherto@17e4144d7ba68f7c3e8c16eece5aed15fd7c2dc8CHART_VERSIONto usegithub.ref_nameinstead of reading from Chart.yaml:Benefits
Tracking
Related to LFXV2-872