-
Notifications
You must be signed in to change notification settings - Fork 203
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
Replaced rustc_serialize with serde #709
Conversation
I hate everything about this, while updating I encountered a compile error in |
I've followed the original json format, but I may have missed something, so whoever reviews make sure they're identical |
This needs to be rebased. Also, I'm not convinced that switching everything at once is the right way to go, see e.g. #459 (comment). The added tests are great, would it be possible to add those in a separate PR? That would avoid blocking them on switching to serde. |
e760b83
to
66df6a4
Compare
Update: Waiting on #743, and then am just going to rewrite this PR entirely |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good once last nit is fixed :)
Co-authored-by: Joshua Nelson <joshua@yottadb.com>
rustc_serialize
serde
andserde_json
Notes:
This is currently broken, as handlebars hates every fiber of my being. I can't figure it out, so I'm hoping one of y'all can figure it out. I've enabled theserde
feature for it and am feeding it aserde
compatible struct, but it says thatPage<T>
doesn't implementSerialize
, when it does.