search

axiomhq / rust-cuckoofilter

Cuckoo Filter: Practically Better Than Bloom (In Rust)
MIT License
271 stars 38 forks source link

Move Serde functionality behind a feature flag #48

Closed JayKickliter closed 4 years ago

JayKickliter commented 4 years ago

This PR moves Serde functionality behind the feature flag serde_support. Note, this PR is a breaking change for anyone already using Serde functionality in this crate.

Closes #42

Still needed

  1. run CI tests with both configurations? This could help prevent silently Serde functionally in future PRs.
CosmicHorrorDev commented 4 years ago

Also CuckooFilter.export() would require the feature since ExportedCuckooFilter is only present with the feature flag.

JayKickliter commented 4 years ago

ExportedCuckooFilter is only present with the feature flag.

It's still there without the flag. Since I added cfg_attr, the only thing that changes is whether or not it derives Deserialize/Serialize.

JayKickliter commented 4 years ago

@LovecraftianHorror sorry for nerd sniping you. I only skimmed the issue, and didn't notice that you were planning on doing this work.

CosmicHorrorDev commented 4 years ago

It's still there without the flag.

Oops, my mistake. Thanks for letting me know.

And don't worry about taking over the issue. I've been pretty busy as is so I'm just glad the functionality got added!