Skip to content
This repository has been archived by the owner on May 6, 2020. It is now read-only.

Update to serde 0.9 #35

Open
dtolnay opened this issue Jan 29, 2017 · 16 comments
Open

Update to serde 0.9 #35

dtolnay opened this issue Jan 29, 2017 · 16 comments

Comments

@dtolnay
Copy link
Contributor

dtolnay commented Jan 29, 2017

Release notes: https://github.com/serde-rs/serde/releases/tag/v0.9.0

One thing we need to figure out is where to handle all the things that currently happen inside MapDeserializer::missing_field.

@RReverser
Copy link

Just FWIW, I've implemented own Serde 0.9 compatible deserializer with support for enums, collections of enums, and other features this deserializer lacked for me here https://github.com/RReverser/serde-xml-rs

@dtolnay
Copy link
Contributor Author

dtolnay commented Feb 17, 2017

This is amazing. @oli-obk how would you feel about handing ownership of serde_xml on crates.io over to @RReverser and abandoning this codebase? IIRC you were pretty pessimistic about the future of this crate (the words were something like "failed experiment").

@oli-obk
Copy link

oli-obk commented Feb 18, 2017

(the words were something like "failed experiment").

yes. this repo would need a rewrite. It is unsalvageable.

I suggest to keep the tests though (or at least take them as a starting point, some of them should probably be broken in a better serializer, but all of them should be implementable)

@RReverser
Copy link

@oli-obk Sure, I can copy-paste your tests to my repo if that's okay with you.

@dtolnay
Copy link
Contributor Author

dtolnay commented Feb 18, 2017

Great. @RReverser up to you whether you would like to keep the repo under your account or take over this one. If you keep yours, please open issues corresponding to every open issue here, and leave a comment on our issues linking to yours. Then I will add you as an owner of serde_xml on crates.io and you can push your version.

@RReverser
Copy link

@dtolnay At the moment I'm seeing quite a bit failed tests from actual serde_xml, when I'll have time, I'd like to fix them before making any further decisions, but will be able to do that only on the next week.

@oli-obk
Copy link

oli-obk commented Feb 20, 2017

Sure, I can copy-paste your tests to my repo if that's okay with you.

sgtm

At the moment I'm seeing quite a bit failed tests from actual serde_xml,

Always verify that they make sense and don't require complex dances in the deserializer impl. It can always be required that the user writes a complex type to satisfy weird xml structures, but you should not strife to support them by default from simple structures. That's one of the problems I've had in this repository.

@RReverser
Copy link

@oli-obk Sure. For example, tuple/unit failures are valid, and I'm working on improving their support, but I don't want, for example, to allow parsing primitives from <wrapper>primitive</wrapper>, as that's not really eligible for round-trip.

@indiv0
Copy link

indiv0 commented Feb 21, 2017

@RReverser Would this then prohibit something like:

type Plaintext = String;
<plaintext>sometext</plaintext>
<plaintext/>

?

Because that's a use-case I need.

@RReverser
Copy link

@indiv0 Yes, because for Rust type is just a transparent alias - when you use it, it's not even a wrapper, but exactly as if you would write String there in the first place.

@indiv0
Copy link

indiv0 commented Feb 23, 2017

@RReverser Alright. How would I go about handling that case then?

(Also I realize this is a bit off-topic for this issue. If you'd like, we could move this discussion to a serde_xml_rs issue)

@RReverser
Copy link

@indiv0 Sure, please do.

@RReverser
Copy link

@oli-obk @dtolnay Looks like I don't have much time to evolve serde-xml-rs as I don't do anything related to XML parsing which motivated me to create it in the first place.

At the same time, this original repo/crate is deprecated, so I thought if you still feel that it's worth it, we can move repo and/or rename crate as was originally suggested.

@oli-obk
Copy link

oli-obk commented Feb 23, 2018

Yea we should do that. I can give you access rights to the serde-xml crate and then you can push your crate there as a 1.0 version.

@RReverser
Copy link

Sounds good. Should we move repo over as well?

@dtolnay
Copy link
Contributor Author

dtolnay commented Feb 23, 2018

I would leave the repo with you. I don't expect XML to become a flagship Serde format (especially if no longer actively developed) and the link from https://serde.rs should make it sufficiently discoverable.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

4 participants