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

Remove #[non-exhaustive] for compatiblity back to rust 1.34.0 #229

Merged
merged 1 commit into from May 10, 2020
Merged

Remove #[non-exhaustive] for compatiblity back to rust 1.34.0 #229

merged 1 commit into from May 10, 2020

Conversation

bpfoley
Copy link
Contributor

@bpfoley bpfoley commented May 9, 2020

(Which is the version in Debian stable and many other distros).

Otherwise the minimum supported Rust version is 1.40.0

While here, add a CI job that tests Rust 1.34.0 builds too

@bpfoley
Copy link
Contributor Author

bpfoley commented May 9, 2020

Note that the MSRV for the different releases of ron is

v0.1 MSRV = Fails to build with any version I could find
v0.1.1 MSRV = 1.31.0 (because of edition=2018)
v0.1.2 MSRV = 1.31.0 ..
v0.1.3 MSRV = 1.31.0 ..
v0.1.4 MSRV = 1.31.0 ..
v0.1.5 MSRV = 1.31.0 ..
v0.1.6 MSRV = 1.31.0 ..
v0.1.7 MSRV = 1.31.0 ..
v0.2 MSRV = 1.31.0 ..
v0.2.1 MSRV = 1.31.0 ..
v0.2.2 MSRV = 1.31.0 ..
v0.3 MSRV = 1.31.0 ..
v0.4 MSRV = 1.31.0 ..
v0.4.1 MSRV = 1.31.0 ..
v0.4.2 MSRV = 1.31.0 ..
v0.5 MSRV = 1.34.0 (because of renaming imports with _)
v0.5.1 MSRV = 1.31.0 (because of edition=2018)
master MSRV = 1.40.0 (because of use of core::convert::TryInto from 1.34.0 and #[non_exhaustive] from 1.40.0)

@bpfoley bpfoley requested a review from kvark May 9, 2020
kvark
kvark approved these changes May 10, 2020
Copy link
Collaborator

@kvark kvark left a comment

Thank you!
bors r+

@GrayJack
Copy link

GrayJack commented May 10, 2020

This change would change behavior when matching, to allow the same behavior as before and support older releases of Rust, the following should be added as a new variant for that enum

#[doc(hidden)]
__Nonexhaustive,

Unless, of course, if the change of behavior was intended.

@kvark
Copy link
Collaborator

kvark commented May 10, 2020

bors r-

@kvark
Copy link
Collaborator

kvark commented May 10, 2020

Thank you for chiming in, @GrayJack !
I thought, this is API breaking anyway, so maybe it doesn't matter that you can match now.
I do agree that following that #[doc(hidden)] variant is a little more future-proof, in case we are stuck with 1.34.0 for long.

@GrayJack
Copy link

GrayJack commented May 10, 2020

Yeah, the breaking change by itself wasn't the problem, the problem, at least for me was that any new variant added would be a breaking change, so it was more about future-proof than anything

(Which is the version in Debian stable and many other distros).

Otherwise the minimum supported Rust version is 1.40.0

While here, add a CI job that tests Rust 1.34.0 builds too
@kvark
Copy link
Collaborator

kvark commented May 10, 2020

bors r=GrayJack

@kvark kvark merged commit f2e7672 into ron-rs:master May 10, 2020
5 of 6 checks passed
@kvark
Copy link
Collaborator

kvark commented May 10, 2020

Bors job list was wrong, I just fixed it, so future PRs should be landing as usual.

@bpfoley bpfoley deleted the exhaust branch May 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants