Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upAdd serde 1.0 support #296
Merged
Conversation
Wallacoloo
added a commit
to Wallacoloo/libfriendship
that referenced
this pull request
Apr 29, 2017
Note: now using a fork from the servo/rust-url repo until servo/rust-url#296 is merged.
| impl Deserialize for De<Url> { | ||
| fn deserialize<D>(deserializer: D) -> Result<Self, D::Error> where D: Deserializer { | ||
| impl<'de> Deserialize<'de> for De<Url> { | ||
| fn deserialize<D>(deserializer: D) -> Result<Self, D::Error> where D: Deserializer<'de> { | ||
| let string_representation: String = Deserialize::deserialize(deserializer)?; |
This comment has been minimized.
This comment has been minimized.
SimonSapin
Apr 30, 2017
Member
With serde 1.0 we might be able to use Cow<'de, str> here instead of String (https://serde.rs/borrow.html) and make one less memory allocation in most cases. But that can be done later.
|
Thanks for this PR! Looks good. As discussed in #295 we’re not quite sure yet how to deal with serde 1.0 in all of Servo’s dependency graph, but that shouldn’t block this. We can always publish other 0.1.x versions of @bors-servo r+ |
|
|
bors-servo
added a commit
that referenced
this pull request
Apr 30, 2017
Add serde 1.0 support This commit is adding support for serde 1.0 and update docs to address #282. Fix #282 Fix #295 Note1: I had to bump the version of `url_serde` to `0.2` as the changes are not backward compatible and will cause custom derive to fail if the users are using `serde 0.9`. Note2: I have decided not to use `remote-derive` serde feature for now. I will look into it more and maybe to another PR later. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-url/296) <!-- Reviewable:end -->
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
mhristache commentedApr 22, 2017
•
edited by larsbergstrom
This commit is adding support for serde 1.0 and update docs
to address #282.
Fix #282
Fix #295
Note1: I had to bump the version of
url_serdeto0.2as the changes are not backward compatible and will cause custom derive to fail if the users are usingserde 0.9.Note2: I have decided not to use
remote-deriveserde feature for now. I will look into it more and maybe to another PR later.This change is