-
Notifications
You must be signed in to change notification settings - Fork 484
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
chore(common): bump MSRV to 1.56 #866
Conversation
Codecov Report
@@ Coverage Diff @@
## main #866 +/- ##
=======================================
- Coverage 66.6% 66.6% -0.1%
=======================================
Files 113 113
Lines 8727 8726 -1
=======================================
- Hits 5820 5813 -7
- Misses 2907 2913 +6
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
I'm confused about the dashmap change (the repository seems to be testing 1.49). In general, in the crates ecosystem bumping the MSRV without bumping the feature version seems to be the de facto standard so calling it out doesn't seem helpful. I would probably just go with 1.57 if that's what's required for time. |
Their
Emm I think we should try to keep MSRV as low as possible. I intend to replace the |
I wonder how useful this form of MSRV support is. For any downstream users that actually want to keep their dependencies up to date, it doesn't seem very meaningful. If our dependencies require a higher MSRV we should just adopt their MSRV and/or push back on their MSRV bumps. Otherwise the result is that MSRV xor bug fixes, which doesn't seem great either. |
I think one use case is users that who are using 1.56 can pin
I am also concerned that simply keeping our MSRV equal to the |
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.
This looks good to me. I agree that this isn't a perfect solution but I don't have a better one at the moment.
Gonna merge it as it is. Feel free to reopen the discussion if needed. |
rust-version
inCargo.toml
spatch_dependencies
to lowerdashmap
andtime
version.dashmap
latest version requires 1.59,time
latest version requires 1.57indexmap
from1.8