-
Notifications
You must be signed in to change notification settings - Fork 468
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
Bump MSRV to 1.36 #504
Bump MSRV to 1.36 #504
Conversation
Almost there - looks like this just needs a |
CI passed. |
Ah, wait, we can remove some build scripts and cfgs: crossbeam/crossbeam-epoch/build.rs Lines 5 to 7 in c239d8f
|
I added the folloing changes:
|
Updated MSRV policy based on @jonhoo's suggestion.
|
Do we want to give explicit MSRV numbers for the different features too? At least where they differ. Or maybe 1.36 covers all features? Do we want CI to check that MSRV holds for all features? |
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.
I have a very minor comment. Thanks for the PR!
@jonhoo I can't understand your latest comment. Would you please rephrase your question?
@jonhoo I think MSRV policy should cover all features (except for the |
@jeehoonkang Ah, sorry, what I meant was that we'll want to make sure that the MSRV covers all features, or if it doesn't, document which features have which MSRV. @stjepang If that's the case, that's awesome! Do we check this on CI at the moment? If not, might be a good thing to add so we don't accidentally break that later on. I think @taiki-e's |
Yup, by using cargo-hack, we can check this with the following one command, and will not miss it if features are added in the future. cargo +1.36.0 hack check --all --each-feature --no-dev-deps --ignore-private --skip nightly |
That seems like a fantastic thing to have in CI! |
527: Check all feature combinations works properly on CI r=stjepang a=taiki-e This adds a check for all feature combinations to CI. *This originally suggested by @jonhoo in #504 (comment) Co-authored-by: Taiki Endo <[email protected]>
Maybe this can fix the CI.
cc #503