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

Added MSRV info to crates #1110

Merged
merged 4 commits into from
Nov 4, 2024
Merged

Conversation

BenFordTytherington
Copy link
Collaborator

Adding an MSRV to crates, in order to enforce an optional clippy lint which prevents newer features breaking things.

Copy link

codecov bot commented Oct 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (f1762f0) to head (7a56f8a).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##              main     #1110    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           71        71            
  Lines        12173     11927   -246     
==========================================
- Hits         12173     11927   -246     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@BenFordTytherington
Copy link
Collaborator Author

MSRVs were found using https://github.com/foresterre/cargo-msrv, but when trying to run cargo test on the project-wide MSRV, it failed. I ran cargo test on 1.77.2 which is the highest MSRV for any of the crates, and it seemed that any rust version before 1.80.0 broke the generation tests. The diff showed that all the output simply didn't happen, so all tests passed except for the generation ones. This may be an issue with my locally installed toolchains however, but worth exploring.

@ahayzen-kdab
Copy link
Collaborator

MSRVs were found using https://github.com/foresterre/cargo-msrv, but when trying to run cargo test on the project-wide MSRV, it failed. I ran cargo test on 1.77.2 which is the highest MSRV for any of the crates, and it seemed that any rust version before 1.80.0 broke the generation tests. The diff showed that all the output simply didn't happen, so all tests passed except for the generation ones. This may be an issue with my locally installed toolchains however, but worth exploring.

For me the generation tests work with 1.77.2, i suspect maybe you didn't have rustfmt installed with that toolchain enabled? As note that we call out to that. Or something similar ?

Cargo.toml Outdated Show resolved Hide resolved
@BenFordTytherington BenFordTytherington marked this pull request as ready for review October 28, 2024 16:57
Cargo.toml Outdated Show resolved Hide resolved
@ahayzen-kdab
Copy link
Collaborator

We appear to be hitting mozilla/sccache#2092 on macOS as we have reached the maximum cache size and it cannot remove old entries. Will need to debug if there is a way around this :-/

ahayzen-kdab
ahayzen-kdab previously approved these changes Nov 4, 2024
Copy link
Collaborator

@ahayzen-kdab ahayzen-kdab left a comment

Choose a reason for hiding this comment

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

LGTM :-)

- Fix a clippy lint in lib.rs
- Pin rust version in all CI jobs that use cargo commands
Copy link
Collaborator

@ahayzen-kdab ahayzen-kdab left a comment

Choose a reason for hiding this comment

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

LGTM :-)

@ahayzen-kdab ahayzen-kdab merged commit 4455c86 into KDAB:main Nov 4, 2024
16 checks passed
@BenFordTytherington BenFordTytherington deleted the rust-msrv branch November 4, 2024 16:20
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.

2 participants