-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add release logic #35
base: main
Are you sure you want to change the base?
Add release logic #35
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this one @miles-w-3 I'll find some time this week to go through and see these changes in action. It's great to see some progress made towards the next release though.
About protecting the charts directory from accidental commits, a .git/hooks/pre-commit
hook could be a good option to prevent direct commits. Something like this might work:
#!/bin/bash
if git diff --cached --name-only | grep -q "^charts/"; then
echo "Direct commits to the charts directory not allowed"
echo "Use the release process instead"
exit 1
fi
Cheers.
python-versions: "${{ matrix.python-versions }}" | ||
- name: "Run nox -s ${{ matrix.session }}" | ||
run: | | ||
nox -s "${{ matrix.session }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@miles-w-3 Should we really delete this workflow? At the moment it only runs the docs build session but we might decide to add other nox sessions for things un-related to the release, like linting, spell checks, etc.
@@ -5,7 +5,7 @@ gh-pages/ | |||
/bundle | |||
/bundle_tmp* | |||
/bundle.Dockerfile | |||
/charts | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure we want an empty line here 😄
@@ -1,14 +1,18 @@ | |||
# Include the Makefile from the ansible/awx-operator repository. | |||
# This Makefile is created with the clone-awx-operator.py script. | |||
include Makefile.awx-operator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@miles-w-3 I've only been able to take a bit of a cursory look at these changes but if we no longer need the makefile from the awx-operator repo should we also update the clone-awx-operator.py
script so it doesn't create it?
Closes #30
A snapshot of what I did:
Create my pages branch
find . -name ".*" -not -name "." -not -name ".." -not -name ".git" -maxdepth 1 -exec rm -rf {} \;
# AWX Operator Helm Release This branch is used to manage github-pages releases for the AWX Operator Helm Chart
git add README.md
git commit -am "chart releaser pages config"
-a
is important to ensure all files are removedgit push --set-upstream origin gh-pages
Create configs in main
git switch main
git switch -c feature/chart-releaser-to-main
.workflows
Remaining design decisions
A huge consequence of this approach is that the
charts
directory can't be gitignored, since chart-releaser uses it to detect changes. A consequence of this is that contributors need to make sure that they don't commit intocharts/
on release. I think a natural solution to this is to make a developmentmake
target for helm generation that goes to a/test
folder, and then have a release make target which goes tocharts
and is used only in the release pipeline. That will keep this directory clean from accidental commits.There's probably another approach we could take of permanently populating
charts
with enough stubs to fool chart-releaser into always wanting to make a release, but I'm not sure if we want to do that either - imo there is value in allowing non-chart-specific commits like readme and other top-level file updates which should not trigger a release.Blockers for merge
I think that the current gh-pages branch is already in a good spot. It points at all of the old releases in the index file, and chart-releaser will seamlessly add releases onto it. @rooftopcellist can probably confirm, but I'm guessing the custom url for the page has already been set up.
Therefore, I don't think there are any remaining blockers beyond figuring out how to limit dumping in
charts/
. In this branch, I set the chart version at 3.0.0, since @oraNod as you pointed out helm recommends using semver.One last thought, we may want a PR validation workflow that generates the chart and checks if there are file changes in
charts/
. If there are, it should make sure that the generated chart has a newversion
.