Skip to content
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 GitHub workflow for making releases #945

Closed
wants to merge 2 commits into from
Closed

Add GitHub workflow for making releases #945

wants to merge 2 commits into from

Conversation

di
Copy link
Member

@di di commented Nov 30, 2022

This PR implements a GitHub workflow to automate publication of Twine releases when GitHub releases are made, using https://github.com/pypa/gh-action-pypi-publish/ to publish to PyPI.

Needs PYPI_TOKEN to be set for the repo in order for this to succeed.

@bhrutledge
Copy link
Contributor

bhrutledge commented Nov 30, 2022

I like the direction of this, but at first glance, it doesn't seem to take into account the discussion in #683. It also doesn't update the documented release process, which includes generating a changelog, and uses an existing GHA job.

@di
Copy link
Member Author

di commented Nov 30, 2022

Thanks, wasn't aware of that issue. I'll read it and update as necessary, can also update docs of course!

- name: publish
uses: pypa/gh-action-pypi-publish@d7edd4c95736a5bc1260d38b5523f5d24338bc25
with:
user: __token__
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not necessary, it's already the default

Suggested change
user: __token__

release-pypi:
needs: [build]
runs-on: ubuntu-latest
permissions: {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think contents: read may be necessary for the artifact download.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, you could drop all the privileges on the workflow level additionally.

if-no-files-found: warn

release-pypi:
needs: [build]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I strongly recommend putting that token in an environment:

Suggested change
needs: [build]
needs: [build]
environment:
name: pypi
url: https://pypi.org/project/twine/

The environment with the name pypi may be created automatically when this workflow runs. It will be empty.
But you can create it upfront and add the secret there. While on that setting page, also enable the approval requirement and a small cool-down timer.

with:
name: built-packages
path: ./dist/
if-no-files-found: warn
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not error out?

# Confusingly, this action also supports updating releases, not
# just creating them. This is what we want here, since we've manually
# created the release that triggered the action.
uses: softprops/action-gh-release@v1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is also possible to create discussions attached to the releases. Example: https://github.com/cherrypy/cheroot/releases/tag/v9.0.0 (there's a button at the bottom). I started doing this in my automations.
But I think, it only works on creation, not updating. The 📢 announcements discussion category should be pre-existing.

name: Release

on:
release:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lately I've been preferring the workflow_dispatch event — it allows me to type in the version number I want and both PyPI and GH releases get created with that.
I can also make Git updates like generating the changelog, bumping the version, making a tag, and pushing that as a part of the workflow, making sure all the steps are automated, lowering the possibility of human errors as much as possible.

permissions:
id-token: write
outputs:
hashes: ${{ steps.hash.outputs.hashes }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see no such step.

name: Build artifacts
runs-on: ubuntu-latest
permissions:
id-token: write
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure nothing uses this. Did you mean to set contents: write for the upload action?

Copy link
Contributor

@bhrutledge bhrutledge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formally requesting changes, per #945 (comment)

@chrysle
Copy link
Contributor

chrysle commented Jun 2, 2023

Can I overtake the PR? I would also configure trusted publishing.

@webknjaz
Copy link
Member

webknjaz commented Jun 2, 2023

@chrysle I'd say go for it and make a new PR — Dustin is a busy man and I doubt he'll have time to rework this PR anytime soon... Though, the maintainers would still need to set up trusted publishing on the PyPI side.

@bhrutledge
Copy link
Contributor

Can I overtake the PR? I would also configure trusted publishing.

@chrysle Thanks for the offer. I'm happy to have you take a pass at it, but I think it's worth noting that it might take awhile to review.

Also, if you haven't already, please read through all of the comments and their links, for context.

@di
Copy link
Member Author

di commented Jun 2, 2023

Can I overtake the PR? I would also configure trusted publishing.

Yes, please do!

@miketheman
Copy link
Member

This looks to be covered by #1047 and can be closed.

@sigmavirus24 sigmavirus24 deleted the release-workflow branch March 4, 2024 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants