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

Specify minimum Rust version 1.57 (fixes #2333) #2334

Merged
merged 6 commits into from Jun 28, 2022
Merged

Conversation

Nutomic
Copy link
Member

@Nutomic Nutomic commented Jun 27, 2022

Tested this, with 1.56 it doesnt compile, but 1.57 works. Only disadvantage is that the older compiler might give worse error outputs, and might have less clippy lints.

@Nutomic Nutomic changed the title Specify minimum Rust version 1.59 (fixes #2333) Specify minimum Rust version 1.57 (fixes #2333) Jun 27, 2022
@Nutomic Nutomic marked this pull request as ready for review June 27, 2022 10:46
@Nutomic Nutomic requested a review from dessalines as a code owner June 27, 2022 10:46
crates/api/Cargo.toml Show resolved Hide resolved
@@ -9,7 +9,7 @@ platform:
steps:

- name: prepare repo
image: clux/muslrust:1.59.0
image: clux/muslrust:1.57.0
Copy link
Member

Choose a reason for hiding this comment

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

Any reason for downgrading? I just checked clux, and they're up to 1.61.0

Copy link
Member Author

Choose a reason for hiding this comment

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

To make sure we dont accidentally introduce a change which requires a newer Rust version. I suppose we could only run cargo check with 1.57, and clippy with latest stable, or something like that.

Copy link
Member

Choose a reason for hiding this comment

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

Yep, I would def like to keep our actual images on the most updated one, there's pry optimizations we don''t want to miss out on.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well the Rust version for release builds is specified in docker/prod/Dockerfile, i didnt change anything about that.

@Nutomic Nutomic force-pushed the min-rust-version branch 2 times, most recently from 7fe1c2b to 18edf21 Compare June 28, 2022 20:33
@Nutomic
Copy link
Member Author

Nutomic commented Jun 28, 2022

I tried to use clux/muslrust:1.61.0 for clippy but that gave weird errors, so i switched to official rust image for that step instead. Now its just a bit weird that clippy gives a wrong warning about unused clone which i dont get locally.

@dessalines dessalines merged commit 587a0de into main Jun 28, 2022
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

2 participants