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

Pin to the older msrv version. #85

Merged
merged 2 commits into from
Oct 11, 2024
Merged

Pin to the older msrv version. #85

merged 2 commits into from
Oct 11, 2024

Conversation

evshary
Copy link
Contributor

@evshary evshary commented Oct 11, 2024

Fix the CI failure
https://github.com/eclipse-uprotocol/up-transport-zenoh-rust/actions/runs/11265771929/job/31328151458

If we use msrv 0.16.x, it needs at least Rust 1.78.0. I guess it will be good to support the minimum version we need.
Therefore, I decide to fix at 0.15.1.

@evshary evshary requested a review from sophokles73 October 11, 2024 02:46
@sophokles73
Copy link
Contributor

IMHO the issue is that you are explicitly setting the rust toolchain to 1.75 using the rust-toolchain.yaml file.
FMPOV this is not really necessary. The msrv check in the nightly build is specifically designed to make sure that your code can actually be compiled using rust 1.75 ...
If you remove the rust-toolchain.yaml file, everthing should work as expected. Alternatively, you could explicitly switch to the stable toolchain before invoking cargo install msrv-check ...

@evshary
Copy link
Contributor Author

evshary commented Oct 11, 2024

IMHO the issue is that you are explicitly setting the rust toolchain to 1.75 using the rust-toolchain.yaml file. FMPOV this is not really necessary. The msrv check in the nightly build is specifically designed to make sure that your code can actually be compiled using rust 1.75 ... If you remove the rust-toolchain.yaml file, everthing should work as expected. Alternatively, you could explicitly switch to the stable toolchain before invoking cargo install msrv-check ...

I remember the reason I added rust-toolchain.yaml is because Zenoh couldn't compile with the latest compiler at that time. Since this is not the case, I agree it's fine to remove it now.

Copy link
Contributor

@sophokles73 sophokles73 left a comment

Choose a reason for hiding this comment

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

LGTM

@sophokles73 sophokles73 merged commit 133fb88 into main Oct 11, 2024
7 checks passed
@sophokles73 sophokles73 deleted the pin_older_msrv_version branch October 11, 2024 13:36
@evshary evshary added the bug Something isn't working label Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants