-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Enable PyPI publishing from GitHub Actions #27563
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
Conversation
6f6216b
to
8f2cc49
Compare
83bb4f8
to
bfdd037
Compare
We do successfully have all the wheels we'd like, I think: I'm going to revert this back to only attempting to publish when there's an actual tag. |
bfdd037
to
4b9b43f
Compare
The remaining question is whether the nightly upload workflow will continue to work, i.e., whether the download procedure there is compatible with actions/artifacts@v4. Do you have any idea about this @matthewfeickert? |
@QuLogic Sadly I think the answer is no, without refactors. On 2023-12-15 @henryiii alerted the IRIS-HEP Slack to this with the following
Actually though, you may be good as @henryiii is awesome and updated
is checked as working! However, things broke for I have yet to go through and fix things for my own projects yet (I've been AFK on holiday and my only laptop died so I have only recently gotten a loaner that I can use) so once I do that (probably next week) I can open a PR for |
with: | ||
name: sdist | ||
name: cibw-sdist |
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.
Ah @QuLogic sorry I didn't review before commenting, but given that you've given distinct names I think everything is fine as you're doing what @henryiii did in https://github.com/scientific-python/cookie/pull/350/files. So I think this is good (without having done it myself). 👍
Pyhf seems to have updated one but not both actions. You have to have them match. |
This looks good to me. I'm not sure what this warning is referring to though, and can't find any other info on it: ![]() @QuLogic do you is this safe to ignore before merging? |
I believe that error was in testing when it was enabled to go further than it usually is for PRs. |
Yes, @ksunden is correct; I was testing out the full workflow and removed all conditionals, so it "failed" at the end because it's not possible to deploy from the fork. |
@dstansby We were just talking about this on the call and I was about to also hit the green button! Feel free to join for the next 30 minutes :) |
Looks like I missed an artifact name in the nightly upload, which should be fixed in #27643. |
PR summary
Also bump the
artifacts
actions to v4.Note that normally the whole
publish
job should be skipped on non-release events, but I want to confirm that theartifacts
download stuff works.PR checklist