refactor: gate sketches with feature flags #120
Conversation
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces per-sketch Cargo feature flags in the datasketches crate, gates each sketch module behind its corresponding feature, and updates integration tests (and xtask test) to run conditionally based on those features.
Changes:
- Add
[features]todatasketchesand gate sketch modules insrc/lib.rsbehind#[cfg(feature = "...")]. - Gate sketch integration tests behind the same feature flags (e.g.,
#![cfg(feature = "theta")]). - Update
xtask testto discover and enable sketch features viacargo_metadata, and addcargo_metadataas a workspace dependency.
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
xtask/src/main.rs |
Enables running workspace tests with the set of features discovered from datasketches via cargo_metadata. |
xtask/Cargo.toml |
Adds cargo_metadata dependency to support feature discovery. |
datasketches/src/lib.rs |
Gates sketch modules (theta, hll, etc.) behind per-sketch Cargo features. |
datasketches/Cargo.toml |
Introduces per-sketch feature definitions (and sets default = []). |
datasketches/tests/theta_sketch_test.rs |
Gates theta integration tests behind feature = "theta". |
datasketches/tests/theta_serialization_test.rs |
Gates theta serialization tests behind feature = "theta". |
datasketches/tests/theta_intersection_test.rs |
Gates theta intersection tests behind feature = "theta". |
datasketches/tests/tdigest_test.rs |
Gates tdigest integration tests behind feature = "tdigest". |
datasketches/tests/tdigest_serialization_test.rs |
Gates tdigest serialization tests behind feature = "tdigest". |
datasketches/tests/hll_update_test.rs |
Gates HLL integration tests behind feature = "hll". |
datasketches/tests/hll_union_test.rs |
Gates HLL union integration tests behind feature = "hll". |
datasketches/tests/hll_serialization_test.rs |
Gates HLL serialization tests behind feature = "hll". |
datasketches/tests/frequencies_update_test.rs |
Gates frequencies integration tests behind feature = "frequencies". |
datasketches/tests/frequencies_serialization_test.rs |
Gates frequencies serialization tests behind feature = "frequencies". |
datasketches/tests/cpc_wrapper_test.rs |
Gates CPC wrapper integration tests behind feature = "cpc". |
datasketches/tests/cpc_update_test.rs |
Gates CPC update integration tests behind feature = "cpc". |
datasketches/tests/cpc_union_test.rs |
Gates CPC union integration tests behind feature = "cpc". |
datasketches/tests/cpc_serialization_test.rs |
Gates CPC serialization tests behind feature = "cpc". |
datasketches/tests/countmin_test.rs |
Gates count-min integration tests behind feature = "countmin". |
datasketches/tests/bloom_serialization_test.rs |
Gates bloom serialization tests behind feature = "bloom". |
Cargo.toml |
Adds cargo_metadata to workspace dependencies. |
Cargo.lock |
Updates lockfile for the new cargo_metadata dependency set. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
|
Thanks for your review @PsiACE! I'm going to merge this and cut a 0.3.0 release candidate in the next Monday. So I'd appreciate it if @notfilippo @ZENOTME can drop a review so that we don't need to go back for hotfix after the RC is out. |
|
Well. I think I can merge this now and prepare the changelog and other setups. And feedback can steer in to break the release process :D |
This closes #32
Some unused warnings and style issues can be fixed later.