-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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: use newer artifact actions #13991
ci: use newer artifact actions #13991
Conversation
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.
SGTM
Do we know how fast things are currently? |
Short of checking the timestamps not really no, maybe just an eye at the build time between this and the last build (which is a terrible metric). Realistically, I should probably just bump all actions used in this PR too |
Eurgh looks like the changes for v4 of the third party delete-artifact require passing it a token which is kinda meh |
I think you can also raise that to 20. SDK works fine with that version. |
@m1ga I think that's coming from the homebrew action which doesn't look to have updated to 20 yet based on their repo, I'll update where we set 16.x to 20.x though, good shout. |
Hi @ewanharris, should we still use this? :) Happy to do so! |
d0e2f7c
to
444d1a4
Compare
@hansemannn would probably be good to do but I remember facing an issue with this. If you're aiming to do a release this week I'd not worry about this PR |
Thanks a lot - yeah, we're doing a 12.5.0.GA today, but we can merge it into master/12.6.0 once all builds. |
5bd37cc
to
e76f5d1
Compare
@hansemannn this is ready now, I went ahead and updated all the actions, squashed the commits and also removed the |
I was reading about the deprecation of v1/v2 of these and noticed the claim about v4 being stonkingly fast, so figured lets give it a look see. I doubt the time taken to upload things is that slow here