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

serde-1.0 #7

Merged
merged 4 commits into from
Mar 6, 2018
Merged

serde-1.0 #7

merged 4 commits into from
Mar 6, 2018

Conversation

termoshtt
Copy link
Contributor

serde-1.0 should be the default serialization option since rustc-serialize is already obsolete.

Note: This is a breaking change. We need to bump up the version to 0.2.

@cuviper
Copy link
Member

cuviper commented Dec 20, 2017

Ah, yes, I forgot to open issues for this. We did it before in the next branch, rust-num/num#305, but we'll need to redo this in the separate repos.

@cuviper cuviper mentioned this pull request Dec 20, 2017
Copy link
Member

@cuviper cuviper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to update the CI scripts, updating the min to 1.13 (if we don't use derive) and removing the part that tests a downgrade to serde-0.7.

Cargo.toml Outdated

[features]
default = ["rustc-serialize"]
default = ["serde"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this to be in the default features.

src/lib.rs Outdated
Ok(Complex::new(re, im))
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to keep the same manual impl, just updated for the newer API.

@termoshtt
Copy link
Contributor Author

I've fix using next branch of num including

  • use rust-1.15
  • Drop rustc-serialize
  • manual impl (not use serde-derive)

@cuviper
Copy link
Member

cuviper commented Dec 20, 2017

Great, thanks! I may tweak that minimum version, but it's fine for now.

I'm not sure when I'll be ready to start merging changes for 0.2, but I'll use this PR when I do.
(It won't be too long -- I'm less afraid of bumping num-complex, compared to num-traits...)

@cuviper
Copy link
Member

cuviper commented Mar 6, 2018

Rebased -- thanks again!

bors r+

bors bot added a commit that referenced this pull request Mar 6, 2018
7: serde-1.0 r=cuviper a=termoshtt

serde-1.0 should be the default serialization option since rustc-serialize is already obsolete.

Note: This is a breaking change. We need to bump up the version to 0.2.
@bors
Copy link
Contributor

bors bot commented Mar 6, 2018

Build succeeded

@bors bors bot merged commit 403357d into rust-num:master Mar 6, 2018
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.

2 participants