search

cat-in-136 / ws2812-esp32-rmt-driver

WS2812 driver using ESP32 RMT for Rust
MIT License
40 stars 21 forks source link

Remove all default features #35

Closed faern closed 3 months ago

faern commented 6 months ago

I know you juuuust published a breaking change. So this request is a bit too late. But I guess there might be a 0.8.0 release at some point in the future. That means you might not want to merge this just now. Unless you really agree with my argument here and go ahead with 0.8.0 very soon.

Would you consider making all features opt-in instead of on by default? There are people (including me) considering "default features" to be a design mistake in Cargo. They make it too easy to pull in stuff you don't need. Worst is when I directly depend on crate foo and I really want its feature some-feature to be disabled. But a transitive dependency also pulls in foo and they have not set default-features = false and involuntarily pull in some-feature in my dependency tree even though that transitive dependency is not even using it!

faern commented 6 months ago

A related question is what the unstable feature really means? What does it mean in the context of this crate? It does not depend on nightly Rust. Do you intend to make API breaking changes under unstable without releasing a semver incompatible version of this crate? I personally have mixed feelings about that. Also again partially because transitive dependencies can enable it and suddenly cause semver breakage anywhere in the dependency tree. The documentation around unstable could be a bit better. And I can add it if you describe it.

faern commented 6 months ago

It is currently unclear why Ws2812Esp32RmtDriver::write is unstable. The documentation for the method does not mention anything about it, or caveats there might be.

cat-in-136 commented 3 months ago

Note to self: some modification of README.md lacking, follow in #46.