-
Notifications
You must be signed in to change notification settings - Fork 249
Remove use of const fn
, add CI tests for MSRV
#481
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
Conversation
363269b
to
a0317de
Compare
Codecov Report
@@ Coverage Diff @@
## master #481 +/- ##
==========================================
- Coverage 54.17% 54.03% -0.15%
==========================================
Files 30 30
Lines 12640 12662 +22
==========================================
- Hits 6848 6842 -6
- Misses 5792 5820 +28
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
b6fbd12
to
f2addb2
Compare
const fn
, add CI tests for MSRVconst fn
, add CI tests for MSRV
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for going the extra mile and making things work even for 1.54 instead of 1.56!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I think, that changes should be done:
- fix key in
Cargo.toml
- set MSRV according to
cargo check
- add a badge to readme
Also, maybe add a test for minimal versions? It seem that proper way to test it is using https://lib.rs/crates/cargo-minimal-versions.
Specifying dependencies with caret seems unnecessary, actually, their are already specified as caret dependencies. I think, that this part can be reverted if that will not fail the build.
Also, it seems that actually not all const
s are unsupported on old rustc
, maybe leave some of them? I don't insist, though.
I assume by this you mean: repeatedly downgrade the toolchain until
I looked around at other projects to see how it is typically done, and it actually seems rather uncommon. |
I mean, do the same steps that you did to find MSRV as 1.54, but run
|
8da81e0
to
0fac212
Compare
const fn
, add CI tests for MSRVconst fn
, add CI tests for MSRV
44fb918
to
0b62061
Compare
It makes the MSRV overly restrictive, and doesn't provide much benefit. Also add an MSRV check to the CI closes tafia#479
But in any case it works fine if I bump it up a bit higher to 1.46.0. |
Hey guys, I ran into this, can I trouble you to tag a release with this fix? 🙏 |
Gentle nudge on this @Mingun — could we get a new release with this fix too? |
Yes, I'll make a release soon. Actually, I plan to make a new release after merging #490, but because it introduces some significant changes, it will be better to make a intermediate release with features, wanted by community now. After all, releases does not happen much 0.26.0 released. |
No description provided.