-
Notifications
You must be signed in to change notification settings - Fork 66
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
Remove the need for an ADMIN_GITHUB_TOKEN #557
Conversation
Nice, thanks! |
example-workflows/prep-release.yml
Outdated
# description: "Set a placeholder in the changelog and don't publish the release." | ||
# required: false | ||
# type: boolean | ||
silent: |
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.
iirc this was left commented so that the input does not show up in the UI as this is more of an advanced feature (discussed in #526)
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.
Okay fair, I'll comment it out
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 a lot @blink1073
I have some minor questions and suggestions.
# The following are needed if you use legacy PyPI set up | ||
# PYPI_TOKEN: ${{ secrets.PYPI_TOKEN }} | ||
# PYPI_TOKEN_MAP: ${{ secrets.PYPI_TOKEN_MAP }} | ||
# TWINE_USERNAME: __token__ |
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 we keep this for now as we don't remove legacy PyPI publication yet?
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 having it in the docs is enough, new projects should be using Trusted Publishing.
# The following are needed if you use legacy PyPI set up | ||
# PYPI_TOKEN: ${{ secrets.PYPI_TOKEN }} | ||
# PYPI_TOKEN_MAP: ${{ secrets.PYPI_TOKEN_MAP }} | ||
# TWINE_USERNAME: __token__ |
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.
Same comment
|
||
<details><summary>Using PyPI token (legacy way)</summary> | ||
- [ ] Create a "release" [environment](https://docs.github.com/en/actions/deployment/targeting-different-environments/using-environments-for-deployment) on your repository and add an `APP_ID` Environment Variable and `APP_PRIVATE_KEY` secret. |
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 we advice to allow that environment only on protected branches?
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.
Good catch, updated
Co-authored-by: Frédéric Collonval <fcollonval@gmail.com>
for more information, see https://pre-commit.ci
I need to merge and iterate so I can release. |
Thanks a lot @blink1073 for finishing up this PR. |
Fixes #505.