-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Improve GitHub release workflow #11754
Conversation
f4718f4
to
23b6a90
Compare
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.
LGTM! Just one question
uses: softprops/action-gh-release@v1 | ||
with: | ||
body_path: scripts/latest-release-notes.md | ||
files: dist/* |
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.
What does this line do (what is in dist/
)?
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.
The sdist and wheel generated by the hynek/[email protected]
action.
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.
Any particular reason to have these artifacts in github? My instincts tell me it's better to have one source of truth (PyPI).
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 it is standard practice, many other plugins follow the same approach.
Note we are publishing exactly the same files that we upload to PyPI, I particularly like it.
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 am still somewhat interested to know why someone would want to download the wheel/sdist from github instead of pypi, but OK :)
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.
Distro packagers do strange things
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 find it neat to have the sdist/wheel in the GitHub release page -- given it is the exact same package as PyPI I don't see problems with it. 😁
23b6a90
to
daf12b2
Compare
This changes the existing script to just generate the release notes and delegate the actual publishing to the `softprops/action-gh-release@v1` action. This allows us to delete the custom code, which failed recently in https://github.com/pytest-dev/pytest/actions/runs/7370258570/job/20056477756.
daf12b2
to
6321b74
Compare
Follow up to pytest-dev#11754, noticed that the latest GitHub release does not contain the attached files. Output log from the action: ``` 🤔 Pattern 'dist/*' does not match any files. ```
Follow up to #11754, noticed that the latest GitHub release does not contain the attached files. Output log from the action: ``` 🤔 Pattern 'dist/*' does not match any files. ```
Follow up to pytest-dev#11754, noticed that the latest GitHub release does not contain the attached files. Output log from the action: ``` 🤔 Pattern 'dist/*' does not match any files. ```
This changes the existing script to just generate the release notes and delegate the actual publishing to the
softprops/action-gh-release@v1
action.This allows us to delete the custom code, which failed recently in https://github.com/pytest-dev/pytest/actions/runs/7370258570/job/20056477756.
In addition, the 2nd commit enables type checking in
scripts/
for additional safety.I will remove the secrets that are no longer needed from the repository after this is merged.