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

Use serde 0.8 #222

Merged
merged 5 commits into from
Aug 17, 2016
Merged

Use serde 0.8 #222

merged 5 commits into from
Aug 17, 2016

Conversation

SuperFluffy
Copy link
Contributor

I updated bigint, complex, and rational to use serde 0.8, and also fixed deserialization and the serde feature as such in the rational crate (didn't add any tests, but it compiles now).

Similar to #196 for num/complex, “use serde;” needed to be removed in num/rational.

@cuviper
Copy link
Member

cuviper commented Aug 15, 2016

The rational fixes are welcome right away. The serde upgrade changes our public interface though, so that's Yet Another Breaking Change. We should do that eventually, but let's separate this.

@cuviper
Copy link
Member

cuviper commented Aug 15, 2016

Hmm, since it didn't require any code change, maybe our serde dependency could use an explicit range. The caret requirement uses strict semver, but we might allow something like >=0.7, <0.9. Then we should be compatible with any user's 0.7.* or 0.8.*. What do you think?

@cuviper
Copy link
Member

cuviper commented Aug 15, 2016

We really should have at least a build test too. Can you add a line to .travis/test_nightly.sh?
(Only nightly just because I know serde won't work with our rust-1.0.0 build.)

@SuperFluffy SuperFluffy changed the title Use serde 0.8 and fix serde use in num/rational Use serde 0.8 Aug 17, 2016
@SuperFluffy
Copy link
Contributor Author

Alright, the PR was split into two, see #223. I am still basing the serde update off of the other one, since building the entire num crate with the serde feature is broken otherwise.

homu added a commit that referenced this pull request Aug 17, 2016
Fix import serde and rational deserialization

Similar to #196 for num/complex, “use serde;” needed to be removed in num/rational.

Also, deserialization of `num/rational` needed to be fixed by adding type annotations.

This is in response to #222 (comment) of issue #222.

Also added a travis line, in response to #222 (comment). Hope it works.
@cuviper
Copy link
Member

cuviper commented Aug 17, 2016

I confirmed that the dual-serde works locally, but could you add another test for this? The existing test you added will grab the latest possible, so that will be some 0.8.*. After that, you can force a downgrade like cargo update -p serde --precise 0.7.0 and build again.

@SuperFluffy
Copy link
Contributor Author

All done, thanks for the extra requests. I finally had a reason to mess around with travis a bit. :-)

@cuviper
Copy link
Member

cuviper commented Aug 17, 2016

Great, thanks!

@homu r+ 4f6f7b3

@homu homu merged commit 4f6f7b3 into rust-num:master Aug 17, 2016
@homu
Copy link
Contributor

homu commented Aug 17, 2016

⚡ Test exempted - status

homu added a commit that referenced this pull request Aug 17, 2016
Use serde 0.8

I updated `bigint`, `complex`, and `rational` to use `serde 0.8`, and also fixed deserialization and the `serde` feature as such in the `rational` crate (didn't add any tests, but it compiles now).

Similar to #196 for `num/complex`, “`use serde;`” needed to be removed in `num/rational`.
@SuperFluffy
Copy link
Contributor Author

What is the process of getting a new version of this (0.1.35?) to crates.io?

@SuperFluffy SuperFluffy deleted the serde_0.8 branch August 17, 2016 21:42
@cuviper
Copy link
Member

cuviper commented Aug 17, 2016

I just need to go through and publish each updated crate. I'll try to do that tonight.

@cuviper
Copy link
Member

cuviper commented Aug 18, 2016

OK, 0.1.35 is out, enjoy!

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