Copilot commented on code in PR #120: URL: https://github.com/apache/datasketches-rust/pull/120#discussion_r3213341135
########## datasketches/Cargo.toml: ########## @@ -34,6 +34,18 @@ keywords = ["sketch", "hyperloglog", "probabilistic"] all-features = true rustdoc-args = ["--cfg", "docsrs"] +[features] +default = [] + +# Each sketch has its own feature, so that users can opt in to only the sketches they need. +bloom = [] +countmin = [] +cpc = [] +frequencies = [] +hll = [] +tdigest = [] +theta = [] Review Comment: `default = []` makes the crate build with *no* sketches enabled unless the caller opts into features, which is a breaking change for existing users and conflicts with issue #32’s requirement that “by default, all sketches should be turned on”. Consider setting `default` to include all sketch features (or introducing an `all` feature that’s included in `default`). ########## CONTRIBUTING.md: ########## @@ -27,7 +27,7 @@ Rustup will read the `rust-toolchain.toml` file and set up everything else autom ```shell cargo version -# cargo 1.85.0 (<hash> 2024-12-31) +# cargo 1.86.0 (<hash> <date>) Review Comment: The `cargo version` example output uses placeholders (`<hash> <date>`), which doesn’t match the actual format users will see and can be confusing when validating their toolchain setup. Consider using a real example string or removing the parenthesized portion entirely. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
