Skip to content

MSRV 1.31 #59

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

Merged
merged 2 commits into from
Mar 10, 2020
Merged

MSRV 1.31 #59

merged 2 commits into from
Mar 10, 2020

Conversation

nastevens
Copy link
Member

CI has been failing for a while because of a change to cfg-if that increased the MSRV but only changed the patch version. It's probably reasonable to update anyway and open the door to Rust 2018 edition.

@rust-highfive
Copy link

r? @posborne

(rust_highfive has picked a reviewer for you, use r? to override)

@nastevens
Copy link
Member Author

@posborne It looks like this repo isn't running CI for PRs right now, it's only building mine because I pushed my branch into the repo. Is that something you could update? I don't appear to have permissions.

@posborne
Copy link
Member

posborne commented Mar 2, 2020

@posborne It looks like this repo isn't running CI for PRs right now, it's only building mine because I pushed my branch into the repo. Is that something you could update? I don't appear to have permissions.

Building for PRs are enabled. Checking on #58 it appears that Travis decided not to build this because it flagged the submission from @pheki as abuse for whatever reason. There are posts around about these heuristics being incorrect in many cases.

I fired off an email to TravisCI as this seems like the only way to correct.

image

@posborne
Copy link
Member

posborne commented Mar 2, 2020

The situation is a little obnoxious; our policy is only to do an MSRV change with a major version. In this case, this was not done in cfg-if. In this case, I don't see big issues here in just changing the CI even though we haven't really broken the previous MSRV so long as users are willing to use an older cfg-if.

See https://github.com/rust-embedded/wg/blob/master/ops/msrv.md

@nastevens
Copy link
Member Author

Building for PRs are enabled. Checking on #58 it appears that Travis decided not to build this because it flagged the submission from @pheki as abuse for whatever reason.

Interesting - I didn't even know that was a thing. I just assumed we'd missing a setting at some point.

The situation is a little obnoxious; our policy is only to do an MSRV change with a major version.

Oh I totally agree - it's obnoxious that cfg-if bumped MSRV like that in a minor version. I thought about limiting the dependency of cfg-if to <0.1.10 or whatever, but the problem then is that users with newer versions of Rust will be getting an outdated dependency.

So there's actually not an MSRV listed in the README right now. Maybe add a section there with Rust 1.31 as the MSRV, and then just wait for the next feature add before bumping the version? Or are you suggesting we say the MSRV is 1.26 but use 1.31 for CI? Not sure I'm a big fan of the latter.

@posborne
Copy link
Member

posborne commented Mar 2, 2020

In this case I'm fine with moving to 1.31 in the documentation and going from there. When we go to do the next release we can evaluate where things are at; if there is nothing else breaking I think we will just write this off as OK given the circumstances.

So, I guess just amend the msrv commit to update the readme (I think most of the readme predates the MSRV RFC) and this one is good to go.

Upstream cfg-if now requires Rust 1.31 to build (despite not having done
a major version bump). Updating our MSRV gets CI building correctly
again.

Signed-off-by: Nick Stevens <[email protected]>
@nastevens nastevens merged commit 6f7792a into master Mar 10, 2020
@nastevens nastevens deleted the msrv-1.31.0 branch March 10, 2020 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants