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.

Sign up for GitHub

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

Jump to bottom

Upgrade to serde 1.0 #142

Merged
merged 3 commits into from
May 1, 2017
Merged

Upgrade to serde 1.0 #142

merged 3 commits into from
May 1, 2017

Conversation

KodrAus
Copy link
Contributor

@KodrAus KodrAus commented Apr 21, 2017

Closes #141

  • Peppered around a few lifetimes for the changes to Deserialize
  • Use collect_str in Serialize to possibly avoid a String allocation when serialising dates

cc: @lifthrasiir

dtolnay
dtolnay reviewed Apr 23, 2017
View reviewed changes
src/datetime.rs Outdated
FLocal: Fn(&str) -> Result<DateTime<Local>, E>,
fixed_from_str: FFixed,
local_from_str: FLocal)
where FUTC: for<'de> Fn(&'de str) -> Result<DateTime<UTC>, E>,
Copy link
Contributor

Choose a reason for hiding this comment

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

This changes isn't necessary right? Fn(&str) should be equivalent.

src/naive/date.rs Outdated
@@ -1495,7 +1495,7 @@ fn test_encodable_json<F, E>(to_string: F)

#[cfg(all(test, any(feature = "rustc-serialize", feature = "serde")))]
fn test_decodable_json<F, E>(from_str: F)
where F: Fn(&str) -> Result<NaiveDate, E>, E: ::std::fmt::Debug
where F: for<'de> Fn(&'de str) -> Result<NaiveDate, E>, E: ::std::fmt::Debug
Copy link
Contributor

Choose a reason for hiding this comment

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

Also not required.

src/naive/datetime.rs Outdated
@@ -1371,7 +1371,7 @@ fn test_encodable_json<F, E>(to_string: F)

#[cfg(all(test, any(feature = "rustc-serialize", feature = "serde")))]
fn test_decodable_json<F, E>(from_str: F)
where F: Fn(&str) -> Result<NaiveDateTime, E>, E: ::std::fmt::Debug
where F: for<'de> Fn(&'de str) -> Result<NaiveDateTime, E>, E: ::std::fmt::Debug
Copy link
Contributor

Choose a reason for hiding this comment

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

Also not required.

src/naive/time.rs Outdated
@@ -1261,7 +1261,7 @@ fn test_encodable_json<F, E>(to_string: F)

#[cfg(all(test, any(feature = "rustc-serialize", feature = "serde")))]
fn test_decodable_json<F, E>(from_str: F)
where F: Fn(&str) -> Result<NaiveTime, E>, E: ::std::fmt::Debug
where F: for<'de> Fn(&'de str) -> Result<NaiveTime, E>, E: ::std::fmt::Debug
Copy link
Contributor

Choose a reason for hiding this comment

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

Also not required.

@dtolnay
Copy link
Contributor

dtolnay commented Apr 23, 2017

Looks good to me!

@fschutt
Copy link

fschutt commented Apr 27, 2017

Any update? @lifthrasiir

@therustmonk
Copy link

I've tried this update: it works fine!
Will it be merged to master?

@lifthrasiir lifthrasiir merged commit 62195be into chronotope:master May 1, 2017
@lifthrasiir
Copy link
Contributor

I had been busy publishing Kailua last week and forgot to merge this. Sorry for delay!

lifthrasiir added a commit that referenced this pull request May 1, 2017
@lifthrasiir
0.3.1: Minor maintenance release, Serde 1.0 support.
dcf1933 
- Serde 1.0 is now supported. (#142)

  Technically this is a breaking change, but the minor version was not
  effective in avoiding dependency breakages anyway (because Cargo
  will silently compile two versions of crates). Provided that this is
  likely the last breakage from Serde, we tolerate
  this more-than-last-minute change in this version.

- `Weekday` now implements `FromStr`, `Serialize` and `Deserialize`.
  (#113)

- Fixed a bug that the leap second can be mapped wrongly
  in the local tz with some conditions. (#130)

- Some changes to the tests to avoid previously known issues.

Note that the actually published version is very slightly different
from the repository because no published version of bincode supports
Serde 1.0 right now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dtolnay dtolnay dtolnay left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@KodrAus @dtolnay @fschutt @therustmonk @lifthrasiir