-
Notifications
You must be signed in to change notification settings - Fork 23
Add playbook and kayobe automation breaking change info #1977
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
base: stackhpc/2025.1
Are you sure you want to change the base?
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.
Code Review
This pull request adds documentation for two breaking changes in Epoxy. My review focuses on the correctness and clarity of the added documentation. I've found a couple of issues: a grammatical error that affects clarity, and a docker build command example that is syntactically incorrect and would fail if executed. I've provided suggestions to fix both of these issues.
bbed8b8 to
7b9c086
Compare
7b9c086 to
9497991
Compare
9497991 to
3b8558f
Compare
| option to specify Beokay to use Python 3.12 as it is not the default. | ||
|
|
||
| For CI, Kayobe Automation image also needs to be rebuilt with Python 3.12. | ||
| Running the workflow from ``.github/workflows/stackhpc-build-kayobe-image.yml`` |
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.
This isn't quite right.
This one you have referenced is just the one we use for SKC.
The generic one that clients use is called build-kayobe-docker-image.yml, it's written out by ansible-collection-kayobe-automation
3b8558f to
257fdac
Compare
Added more detail about two breaking changes on Epoxy. 1. Any playbook reference other than symlinks can break with new subdirectories of playbooks. 2. Kayobe Automation image needs to be rebuilt with Python 3.12.
257fdac to
4e17510
Compare
Co-authored-by: Alex Welsh <112560678+Alex-Welsh@users.noreply.github.com>
|
LGTM, CI is just a bit broken at the moment |
Added more detail about two breaking changes on Epoxy.