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

ci: use newer artifact actions #13991

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ewanharris
Copy link
Collaborator

@ewanharris ewanharris commented Feb 14, 2024

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

Copy link
Contributor

@cb1kenobi cb1kenobi left a comment

Choose a reason for hiding this comment

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

SGTM

@cb1kenobi
Copy link
Contributor

Do we know how fast things are currently?

@ewanharris
Copy link
Collaborator Author

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

@ewanharris
Copy link
Collaborator Author

Eurgh looks like the changes for v4 of the third party delete-artifact require passing it a token which is kinda meh

@m1ga
Copy link
Contributor

m1ga commented Feb 15, 2024

Node.js 16 actions are deprecated. Please update the following actions to use Node.js 20

I think you can also raise that to 20. SDK works fine with that version.

@ewanharris
Copy link
Collaborator Author

@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.

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.

None yet

3 participants