pangeo-data / helm-chart

Pangeo helm charts
https://pangeo-data.github.io/helm-chart/
21 stars 26 forks source link

Bump chartpress to 0.6.* and maintenance of chart publishing logic #114

Closed consideRatio closed 4 years ago

consideRatio commented 4 years ago

Summary

This PR is mainly a PR to ensure we don't get stuck in an old version of chartpress and help to make future work using chartpress easier.

About

This repo's currently used chartpress (0.3.2) will version and name development releases differently to the most recent chartpress (0.6.0). The most recent chartpress determines the chart version using the latest git tag of a commit on the current branch, the number of commits since it, and the git hash, as described here.

With this PR we transition away from having dates in the chart version. We go from 20.01.03-d9f0cde, to <latest tag>-n<# commits since latest tag>.h<git hash of commit> or <latest tag> if the current commit was tagged and --long wasn't provided. If we want to have the date in the version still, we need to manually provide --tag with both date and hash whenever we use chartpress.

Important

Whatever we do, I think we should ensure Helm 3 compatibility by having valid SemVer 2 versions. Valid SemVer 2 versions require all numerical fields to not start with a leading zero, and that only one separating - is used among other things. This means that 20.01.13-asdf123 for example is invalid because the 01 representing january is purely numerical with a leading zero.

Since the latest tag is used, we should make chartpress strip leading v characters such as in v0.2.2 as that would make an invalid SemVer 2 version. This is currently not done in chartpress so I suggest start tagging valid SemVer 2 versions going onwards while I work to make chartpress support use of v prefix in a tag.

consideRatio commented 4 years ago

@jhamman here is a new attempt ready for review! The key motivation for getting this landed is long term sustainability and future Helm 3 compatibility.

jhamman commented 4 years ago

This all looks good to me. @jacobtomlinson and @rabernat were at the center of our initial setup so I'd like to give them a chance to review here. If I don't hear anything by the end of Thursday, I'll go ahead and merge.

jhamman commented 4 years ago

I went ahead and merged this. We'll watch the deploy to see how it goes. Thanks @jacobtomlinson for the review and @consideRatio for the PR.

consideRatio commented 4 years ago

I'll fix an issue, it did not deploy "because it was not a tagged commit"