-
Notifications
You must be signed in to change notification settings - Fork 64
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
ci: allow auto releasing to pypi #154
Conversation
.github/workflows/release.yaml
Outdated
python -m twine check dist/* | ||
|
||
- name: Publish package distributions to PyPI | ||
if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags') |
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 believe this if
is not necessary, given the trigger is already push on tags?
on:
push:
tags:
- '*'
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 copied it from the default Jazzband template. I'll read more about it to confirm.
https://github.com/jazzband/django-auditlog/blob/master/.github/workflows/release.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.
@nicoddemus Should I use this event instead?
on:
release:
types: [published]
https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#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.
Yeah, I think using on release
and ditching the if statement is the best and simplest approach.
https://docs.github.com/en/actions/learn-github-actions/contexts#github-context
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.
Never used this option, but seems reasonable to require a release rather than a direct tag.
Other repos might do the other way around: push a tag, and one of the things done at the end of the workflow after uploading the package to PyPI is creating a 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.
3.7 is eol
Co-authored-by: Thomas Grainger <[email protected]>
Just occurred to me we should have a |
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 agree that a RELEASING.rst file would be great.
But this looks great! Thanks for looking into it.
if: github.repository == 'pytest-dev/pytest-timeout' | ||
|
||
permissions: | ||
id-token: write # IMPORTANT: this permission is mandatory for trusted publishing |
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 think the publish action recommends building without this permission to minimise access to this permission. I guess that means adding a separate job for the building and passing the build output along as artefacts?
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.
If @aqeelat wants to go with that route, see how pytest-mock does it: https://github.com/pytest-dev/pytest-mock/blob/fcde66b13d09cb5507fe7e4f35e171adaf22994d/.github/workflows/deploy.yml#L13-L25
(we also build the package using @hynek's excellent build-and-inspect-python-package
action).
since pypi seems to recommend an environment for this, i've created a |
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.
Couple of minor suggestions, very nice to see Trusted Publishing being used here :)
pytest-dev is not going to fork this and means it's more re-usable Co-authored-by: Hugo van Kemenade <[email protected]>
Otherwise we have to keep changing this. Co-authored-by: Hugo van Kemenade <[email protected]>
I switched the repo branch from master to main and somehow this got auto-closed. I also can't edit the target branch anymore in the github UI nor re-open it. @aqeelat could you please re-open this PR? It would be really nice to have. |
No description provided.