Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion hack/helm-build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ for chart_dir in deploy/charts/*/; do

cp "${chart_dir}Chart.yaml" "${chart_dir}Chart.yaml.bak"
sed -i.tmp "s/^version:.*/version: $CHART_VERSION/" "${chart_dir}Chart.yaml"
sed -i.tmp "s/^appVersion:.*/appVersion: $CHART_VERSION/" "${chart_dir}Chart.yaml"
sed -i.tmp "s/^appVersion:.*/appVersion: v$CHART_VERSION/" "${chart_dir}Chart.yaml"
Comment on lines 35 to +36

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I vote for a cleanup:

  1. Rename CHART_VERSION in the image workflow to APP_VERSION. Also do not strip the leading v off it anymore (i.e. instead of running this script with CHART_VERSION=0.8.1, it should be run with APP_VERSION=v.0.8.1).
  2. Strip the leading v here in this script to form a valid version for the Helm chart.
  3. Replace version and appVersion in the Chart.yaml with 0.0.0 to indicate that their values are entirely irrelevant, should never be touched and would be overwritten with every kube-bind release anyway. Ideally a small comment # updated by helm-build.sh during the build is placed somewhere in there, too.

My reasoning is that my brain finds the thought

the chart version is used as the app version

more confusing than

the app version is also used as the chart version

^^ Especially since we're not actually maintining a dedicated version just for the chart, we derive it from the appVersion (the Git tag).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do this, wdyt @mjudeikis?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to contribute. I dont see this complex, so if you have ideas how to make it better - take a stab.

rm -f "${chart_dir}Chart.yaml.tmp"

"$HELM" package "$chart_dir" --version "$CHART_VERSION" --destination ./bin/
Expand Down