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: replace toolchain #1156

Conversation

realeinherjar
Copy link
Contributor

Description

This replaces actions-rs/toolchain for dtolnay/rust-toolchain in all CI runs.

Notes to the reviewers

Following the discussion in rust-bitcoin/rust-bitcoin#2113,
actions-rs/toolchain is an "Unofficial GitHub Actions for Rust programming language" which has only one person in the organization, @svartalf, which appears to be MIA for a long time.
People are asking in vain for these fixes, check actions-rs/toolchain#221 (almost a year long).

There are a bunch of warnings in CI, e.g. https://github.com/bitcoindevkit/bdk/actions/runs/6443147276/job/17499339102#step:6:71.

dtolnay/rust-toolchain is by a well-established Rust developer, and it is well-maintaned (and has more stars than actions-rs/toolchain.

Also rust-bitcoin already replaced actions-rs/toolchain with dtolnay/rust-toolchain in most every place

Changelog notice

Replaced actions-rs/toolchain with dtolnay/rust-toolchain in CI.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

@@ -12,19 +12,17 @@ jobs:
rust:
- version: stable
clippy: true
- version: 1.57.0 # MSRV
- version: "1.57.0" # MSRV
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yaml is complicated, things like 1.60.0 will be rounded to 1.6.
As a best practice use strings.
Also check YAML Hell

Comment on lines +120 to 122
- uses: dtolnay/rust-toolchain@stable
with:
# we pin clippy instead of using "stable" so that our CI doesn't break
# at each new cargo release
toolchain: "1.67.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
- uses: dtolnay/rust-toolchain@stable
with:
# we pin clippy instead of using "stable" so that our CI doesn't break
# at each new cargo release
toolchain: "1.67.0"
# we pin clippy instead of using "stable" so that our CI doesn't break
# at each new cargo release
- uses: dtolnay/[email protected]

This could be simplified to this if desired

@realeinherjar realeinherjar force-pushed the einherjar/rust-toolchain branch from 34f8a5f to 12e15f5 Compare October 8, 2023 12:49
@LLFourn
Copy link
Contributor

LLFourn commented Oct 9, 2023

ConceptACK

@notmandatory
Copy link
Member

If the team is generally on board with switching CI to a Nix with #1165 do we still need this PR?

@realeinherjar
Copy link
Contributor Author

No, this is supersed by #1165.

@notmandatory notmandatory removed this from the 1.0.0-alpha.3 milestone Nov 13, 2023
@storopoli storopoli mentioned this pull request Jan 6, 2024
45 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants