Update PR template to not include bumping the chart version
Pull Request
Description of the change
Removes the requirement for bumping the Chart version in PRs.
Benefits
Collecting multiple PRs before making a new release and also avoid rebasing PRs so often.
Possible drawbacks
None
Applicable issues
Additional information
Discussed in the NC Containers public chat with @tvories
Checklist
- [x] DCO has been signed off on the commit.
This will remove the requirement to set the chart version, but will that prevent the workflow from trying to release when a PR is merged?
Oh I haven't though of that. Maybe let's only run the release workflow on tags, then we can just tag the commit that bumps the version?
Would that even work? AFAICT chart-releaser creates the tag and the github release already
I am sure there is a modification to our workflow that we can do to allow us to only publish a chart release on a tag. I think that's all built into the way we currently have it set up, though, so I am thinking we will need to revamp the whole process before we can remove the chart bump requirement
I think we can change this:
https://github.com/nextcloud/helm/blob/a491977458b5111cc0afd06386aa63ef46720f4c/.github/workflows/release.yaml#L4-L11
to just be:
on:
push:
tags:
- "*.*.*"
as the release job that runs is just doing:
- document workflow
- fetch history
- install helm
- add chart dependencies
- run chart releaser
I don't think anything else relies on this, because linting will still run on PRs. This would actually help the other issue #339, however there is one thing I need to test before I can say for sure it's good to go:
In the chart release github action they check for changes since latest tag here:
Code block for checking for chart changes since latest tag
echo 'Looking up latest tag...'
local latest_tag
latest_tag=$(lookup_latest_tag)
echo "Discovering changed charts since '$latest_tag'..."
local changed_charts=()
readarray -t changed_charts <<< "$(lookup_changed_charts "$latest_tag")"
if [[ -n "${changed_charts[*]}" ]]; then
install_chart_releaser
rm -rf .cr-release-packages
mkdir -p .cr-release-packages
rm -rf .cr-index
mkdir -p .cr-index
for chart in "${changed_charts[@]}"; do
if [[ -d "$chart" ]]; then
package_chart "$chart"
else
echo "Chart '$chart' no longer exists in repo. Skipping it..."
fi
done
release_charts
update_index
else
echo "Nothing to do. No chart changes detected."
fi
If it runs on new tags, and the tag it pulls is always the latest tag, it will always exit instead of finishing up it's tasks, including packaging the chart 🤦. If that's the case though, the solution there is to pass in skip-packaging, which skips that bit and continues onto the rest, but then we need to run our own packaging step, so in their code they're doing cr package "$chart" --package-path .cr-release-packages which is the equivilent of helm package --destination .cr-release-packages, so, if in my testing I see that on tag doesn't work, we could also in the following changes to release.yaml:
# ref: https://helm.sh/docs/helm/helm_package/#helm-package
# (destination is set for chart-releaser job step below to know where to check)
- name: Package chart
run: |
helm package --destination .cr-release-packages
- name: Run chart-releaser
uses: helm/[email protected]
with:
charts_repo_url: https://nextcloud.github.io/helm
# ref: https://github.com/helm/chart-releaser-action#inputs
skip_packaging: true
env:
CR_TOKEN: "${{ secrets.GITHUB_TOKEN }}"
Let me test on a fork first though. Sorry for the info dump. It was just a lot of context 😅