petgraph / fixedbitset

A simple bitset container for Rust
https://docs.rs/fixedbitset/
Apache License 2.0
124 stars 47 forks source link

add optional serde support #43

Closed zhengyi-yang closed 3 years ago

zhengyi-yang commented 4 years ago

40

jrraymond commented 4 years ago

Can you add add some tests?

zhengyi-yang commented 4 years ago

@jrraymond I wonder if it's necessary to add tests, because this totally relies on the automatic implementation of serde. Since FixedBitSet only contains a Vec<u32> and a usize, it should be okay as long as the crate serde has been properly tested.

jrraymond commented 4 years ago

I think it's worth it to add a few tests because they are relatively easy to add.

The tests in serde verify that Vec<u32> and usize serialize/deserialize correctly. A test would verify a FixedBitSet X serializes to bytes Y. This test has value because if the layout of FixedBitSet changes, for example the block size changes to u64, then FixedBitSet X might serialize to bytes Y'. This might be acceptable, but a test would make such an API change obvious.

zhengyi-yang commented 4 years ago

No problem.