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

Bump MSRV to 1.36 #504

Merged
4 commits merged into from May 20, 2020
Merged

Bump MSRV to 1.36 #504

4 commits merged into from May 20, 2020

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented May 19, 2020

Maybe this can fix the CI.

cc #503

@taiki-e taiki-e requested a review from a user May 19, 2020 21:10
@ghost
Copy link

ghost commented May 19, 2020

Almost there - looks like this just needs a cargo fmt :)

@taiki-e
Copy link
Member Author

taiki-e commented May 19, 2020

CI passed.

@taiki-e
Copy link
Member Author

taiki-e commented May 19, 2020

Ah, wait, we can remove some build scripts and cfgs:

if cfg.probe_rustc_version(1, 31) {
println!("cargo:rustc-cfg=has_min_const_fn");
}

@taiki-e
Copy link
Member Author

taiki-e commented May 19, 2020

I added the folloing changes:

  • Remove has_min_const_fn cfg and remove a build script of crossbeam-epoch.
  • Use std::mem::MaybeUninit directly and remove maybe-uninit dep.

README.md Outdated Show resolved Hide resolved
@taiki-e
Copy link
Member Author

taiki-e commented May 19, 2020

Updated MSRV policy based on @jonhoo's suggestion.

Crossbeam supports stable Rust releases going back at least six months, and every time the minimum supported Rust version is increased, a new minor version is released. Currently, the oldest supported version is 1.36.

@jonhoo
Copy link
Contributor

jonhoo commented May 19, 2020

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?

Copy link
Contributor

@jeehoonkang jeehoonkang left a 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?

crossbeam-skiplist/README.md Outdated Show resolved Hide resolved
@ghost
Copy link

ghost commented May 20, 2020

@jonhoo I think MSRV policy should cover all features (except for the nightly feature, of course).

@ghost ghost merged commit 321110b into crossbeam-rs:master May 20, 2020
@taiki-e taiki-e deleted the msrv branch May 20, 2020 09:16
@jonhoo
Copy link
Contributor

jonhoo commented May 20, 2020

@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 cargo-hack may be able to help with that.

@taiki-e
Copy link
Member Author

taiki-e commented May 20, 2020

I think @taiki-e's cargo-hack may be able to help with that.

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

@jonhoo
Copy link
Contributor

jonhoo commented May 20, 2020

That seems like a fantastic thing to have in CI!

bors bot added a commit that referenced this pull request Jun 5, 2020
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 <te316e89@gmail.com>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants