mozilla / mentat

UNMAINTAINED A persistent, relational store inspired by Datomic and DataScript.
https://mozilla.github.io/mentat/
Apache License 2.0
1.65k stars 115 forks source link

Edn serde support #726

Closed victorporof closed 6 years ago

victorporof commented 6 years ago

Fixes https://github.com/mozilla/mentat/issues/708

Simple naive implementation of serde serialization/deserialization for edn values. This is a naive approach, relying on serde's defaults for the internal shape.

We can take this much further, but I'm bit wary of allocating more time on this without an active consumer. Points of improvement are around making the json representation of edn more ergonomic, and adding some helpers to stringify/parse edn into/out of json without having to go through serde_json.

I had to update the num crate to use its serde support. Splitting it into num-traits and num-bigint ended up the way I did it, but there's probably a way to rely on the num mothership crate if necessary.

I also enabled serde-supported tests on travis, they weren't before.

victorporof commented 6 years ago

@ncalexan IIRC I had to update num to get access to its serde support for bigint. I think I can update num everywhere to get this if desired. Splitting the num dependency into the 2 smaller crates isn't required, just how I ended up doing it.

If updating num everywhere is acceptable, I believe it's trivial to revive this PR. Otherwise we can shelve it.

ncalexan commented 6 years ago

@ncalexan IIRC I had to update num to get access to its serde support for bigint. I think I can update num everywhere to get this if desired. Splitting the num dependency into the 2 smaller crates isn't required, just how I ended up doing it.

OK -- thanks, that's helpful.

If updating num everywhere is acceptable, I believe it's trivial to revive this PR. Otherwise we can shelve it.

Let's shelve this for now. As you say, there's no pressing consumer; and it's a little awkward until chrono rolls forward. Thanks for teaching me some things, though!