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: update actions to run on Node 16 #1158

Merged
merged 5 commits into from
Nov 14, 2022

Conversation

miles170
Copy link
Contributor

@miles170 miles170 commented Nov 2, 2022

Partial fix for #1156.

  1. update actions/checkout to v3
  2. remove use of actions-rs/cargo

    actions-rs is currently inactive and has an issue like Node.js 12 actions are deprecated. Please use Node.js 16 actions-rs/cargo#216

  3. remove use of actions-rs/toolchain

    actions-rs is currently inactive and has an issue like Node.JS 12 deprecation on GitHub Actions actions-rs/toolchain#219

  4. add use of Swatinem/rust-cache

All tests are passed, as you can see here. (https://github.com/miles170/fd/actions/runs/3457767586)

Copy link
Collaborator

@tmccombs tmccombs left a comment

Choose a reason for hiding this comment

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

One thing that I think we lose from using the actions-rs actions is caching between different builds. I don't know how much of a difference that would make for this.

.github/workflows/CICD.yml Outdated Show resolved Hide resolved
.github/workflows/CICD.yml Outdated Show resolved Hide resolved
actions-rs is currently inactive and has an issue like actions-rs/cargo#216
actions-rs is currently inactive and has an issue like actions-rs/toolchain#219
@miles170
Copy link
Contributor Author

miles170 commented Nov 2, 2022

Check latest CI tests here. (https://github.com/miles170/fd/actions/runs/3376050377)

@miles170
Copy link
Contributor Author

miles170 commented Nov 2, 2022

One thing that I think we lose from using the actions-rs actions is caching between different builds. I don't know how much of a difference that would make for this.

I checked the code and it never uses actions-rs/toolchain rustc_hash before.

@msfjarvis
Copy link

You could use https://github.com/Swatinem/rust-cache for caching

@miles170
Copy link
Contributor Author

miles170 commented Nov 4, 2022

@tmccombs Would you like to let me try rust-cache in this PR or issue another PR?

@tmccombs
Copy link
Collaborator

tmccombs commented Nov 4, 2022

I'm fine with either.

@miles170
Copy link
Contributor Author

miles170 commented Nov 4, 2022

  1. CI first runs without cache.
  2. CI second runs with cache.

We must place Rust cache after Install cross otherwise, Post Rust cache calculated hash will mismatch.
See https://github.com/Swatinem/rust-cache/blob/master/src/config.ts#L94-L102

.github/workflows/CICD.yml Outdated Show resolved Hide resolved
.github/workflows/CICD.yml Outdated Show resolved Hide resolved
Copy link
Owner

@sharkdp sharkdp left a comment

Choose a reason for hiding this comment

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

Thank you very much! Just a minor question

@miles170
Copy link
Contributor Author

Check latest CI tests here. (https://github.com/miles170/fd/actions/runs/3457767586)

Copy link
Owner

@sharkdp sharkdp left a comment

Choose a reason for hiding this comment

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

thank you

@sharkdp sharkdp merged commit 45cb15d into sharkdp:master Nov 14, 2022
@miles170 miles170 deleted the ci-update-actions branch November 14, 2022 06:12
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.

4 participants