-
Notifications
You must be signed in to change notification settings - Fork 30
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
Release flow #1019
Release flow #1019
Conversation
once this has had an initial review and people are happy with the approach I'll also update the docs with this explanation, but I wanted to get some feedback on the idea first. |
Oh, also, by it's very nature, this is pr is very hard to test, but I have developed this using this repo: https://github.com/savente93/ci-playground There things can be inspected and tested as much as I could figure out, so if reviewer wants to test/play with it first feel free to do it there. |
Seems like you can trigger workflows from workflows? Max 3 it seems in the notes. https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflow_run |
I also do not understand your note about publishing to docker and pypi on every push to a release branch. Pypi at least does not support removing artifacts, I can't remember if testpypi does. But there should be some cleanup if you push a new version of the artifact. |
I think pixi might be able to also take on the release to conda which would mean we don't have to wait for the feedstock to wake up which would be nice, but this probably requires more research and development so I'm leaving this off for now. I've tested it as well as possible, but given that we can only really test it by actually releasing, it will have to be made more robust over time. I did add as much detail as I thought was appropriate to the docs, and the workflows are (hopefully) fairly well documented, so I think it's good enough for now. |
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.
Most of it looks good, only the branch matching comment is critical
on: | ||
pull_request: | ||
types: | ||
- closed |
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.
Should this not also include some filter on the release branch naming convention? Now this will also trigger on any PR being closed if I understand correctly.
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.
oh that's a very good catch! (that's the downside of working in a simplified test environment I guess 😅). Will fix
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.
I've fixed this now, though thinking about it, if this ran on a random PR, there wouldn't be any artifacts for it to download so it wouldn't have succeeded anyway. I guess that's one upside to doing it the way I have
git tag "v$RELEASE_VERSION" | ||
# post release stuff so we don't forget | ||
sed -i "s/v$RELEASE_VERSION/Unreleased\n==========\n\nNew\n---\n\nChanged\n-------\n\nFixed\n-----\n\nDeprecated\n----------\n\nv$RELEASE_VERSION/" docs/changelog.rst | ||
sed -i 's/__version__.*=.*"\(.*\)".*/__version__ = "\1.dev0"/' hydromt/__init__.py |
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.
__version__
seems hardcoded here?
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.
I get why you think that but it's not. In sed \1
is the code for "first capture group" so this will get substituted with what was captured in the (.*)
earlier in the command
5. After the PR is merged, a action should start (though it will not show up under the PR itself) that will publish the latest artifacts created to their respective platform. After this, a bot will add a final commit to the `main` branch, setting the hydromt version back to a dev version, and adding new headers to the `docs/changelog.rst` for unreleased features. It will also create a tag and a github release for you automatically. The release is now done as far as this repo is concerned. | ||
6. The newly published PyPi package will trigger a new PR to the `HydroMT feedstock repos of conda-forge <https://github.com/conda-forge/hydromt-feedstock>`_. | ||
Here you can use the comment posted to the release PR to see if the `meta.yml` needs to be updated. Merge the PR to release the new version on conda-forge. | ||
7. celebrate the new release! |
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.
Still looking forward to this :)
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.
celebrating the new release?
Co-authored-by: Jaap Langemeijer <33715902+Jaapel@users.noreply.github.com>
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.
👍
Issue addressed
Fixes #972
Explanation
I originally phrased this issue as "migrate release flow to pixi" because I was under the impression that pixi had a
publish
command. it does have aupload
command for conda, which I still want to investigate. Perhaps this can be a more efficient way of dealing with the feedstock but given that this still needs to be investigated I decided to leave it out of scope for this one. Currently the release flow happens entirely within github actions (which has some limitations that I'll get to in a second) and I could move these to the pixi tasks, depending on what people prefer. The upside to doing it within github actions is that it generalises easier to projects which do now use pixi yet, but whether that trade off is worth it I'll leave up to the reviewer.With the changes in this PR the release flow will go as follows:
prepare release
workflow on github. This will:a. Create a release branch with the correct new version number
b. update the
hydromt/__init__.py
anddocs/changelog.rst
with the new version and headers as necessaryc. Create a PR with these changes
a. docs
b. docker image (incl one for binder integration)
c. pypi package
(d. conda package is excluded here because that has to happen externally through the feedstock)
unreleased
header indocs/chagelog.rst
and setting the version inhydromt/__init__.py
back to a dev version.Hope this was a good enough explanation, I know it's a bit hairy on the back end, but it makes the process of actually creating a release much more automated and inspect able I think. It's a very "set it and forget it" setup which I like. Let me know if any additional clarification is required
General Checklist
main