Skip to content

Make futures-util and futures-sink optional #4197

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thetek42
Copy link

This PR makes futures-util and futures-sink within embassy-time and embassy-sync optional dependencies that can be enabled with feature flags. The flags are enabled by default as to hopefully not break to many people's code after updating their dependencies :)

@Dirbaio
Copy link
Member

Dirbaio commented May 14, 2025

What's the motivation, compile time? Have you measured how much this improves compile time?

@thetek42
Copy link
Author

thetek42 commented May 14, 2025

Yup, that's it. Compile time and not having unused crates in the dependencies. I haven't gotten around to measuring it, but even small improvements are helpful.

@Dirbaio
Copy link
Member

Dirbaio commented May 14, 2025

I don't think we should add Cargo features unless the compile time improvement is very big. Adding features makes libs harder to use.

If you make them non-default, users will find something that the docs says should be there is not there, then they have to hunt down what feature enables it and add it.

If you make them default all it takes is one crate to forget default-features=false (like all crates using embassy-sync in this very repo :) ) and the compile time improvement is gone.

@thetek42
Copy link
Author

Good point. I would still be in favour of it, but do with this what you want ^^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants