fix: reduce MSRV to 1.70 and add CI#4
Conversation
WalkthroughThe updates introduce Rust 1.70 as the minimum supported version in the project manifest and ensure CI checks run on both stable and 1.70 toolchains. Internally, a floating-point to bit conversion now uses Changes
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/lib.rs (1)
113-115: Documentunsafetransmute and tighten its scopeUsing
std::mem::transmuteinside aconst fnis a pragmatic workaround becausef64::to_bitsisn’tconstin Rust 1.70.
However, everyunsafeblock should state why it is sound to avoid accidental misuse later.- // TODO: use x.to_bits() with MSRV 1.83 - let xbits: u64 = unsafe { std::mem::transmute::<f64, u64>(x) }; + // SAFETY: `f64` and `u64` have identical size and alignment; the bit-pattern + // copy preserves the exact IEEE-754 representation. This is equivalent to + // `f64::to_bits()` which is not `const` on Rust 1.70. Replace with + // `x.to_bits()` once the MSRV ≥ 1.83. + let xbits: u64 = unsafe { std::mem::transmute(x) };(Using the turbofish is redundant when the types are evident.)
Consider gating this behind a helper
const fn to_bits_constso the unsafe block is written once and removed easily when the MSRV is raised.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
.github/workflows/ci.yml(1 hunks)Cargo.toml(1 hunks)src/lib.rs(1 hunks)
🔇 Additional comments (2)
Cargo.toml (1)
4-4: MSRV field correctly addedExplicitly pinning
rust-version = "1.70"is the right way to make Cargo enforce the lowered MSRV at build time.
No further action needed..github/workflows/ci.yml (1)
21-21: Add Rust 1.70 to the test matrix as wellOnly the “check” job compiles on 1.70; the “test” job still runs on
stableonly. This means logic errors that slip pastcargo check(e.g. failed assertions, panics, feature-gated behaviour) are not exercised on the MSRV toolchain.- rust: [stable] + rust: [stable, 1.70]Mirroring the same matrix you defined for the “check” job keeps compilation and execution coverage in sync.
Likely an incorrect or invalid review comment.
|
Hi @LDeakin, thanks for the PR! If you could please fix the merge errors I'd be happy to merge. |
|
Thanks @EricLBuehler, hopefully all is fixed on the CI side now |
|
Thank you! |
|
Thanks @EricLBuehler! Rust 1.70 is quite old, but once you have a release out with |
2 participants
Nice crate!
33c1797 (0.2.1) raised the MSRV to 1.83. This reduces it back to 1.70 (hopefully), sets an MSRV, and adds CI for it.
Summary by CodeRabbit
Chores
Bug Fixes