search

rust-embedded / rust-sysfs-gpio

A Rust Interface to the Linux sysfs GPIO interface (https://www.kernel.org/doc/Documentation/gpio/sysfs.txt)
Apache License 2.0
383 stars 45 forks source link

MSRV 1.31 #59

Closed nastevens closed 4 years ago

nastevens commented 4 years ago

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 commented 4 years ago

r? @posborne

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

nastevens commented 4 years ago

@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 commented 4 years ago

@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 https://github.com/rust-embedded/rust-sysfs-gpio/pull/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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.