xtra icon indicating copy to clipboard operation
xtra copied to clipboard
verified publisher icon Restioson

Simplify feature dependencies

Open thomaseizinger opened this issue 3 years ago • 5 comments

  1. We only use futures-util's alloc feature in the tests.
  2. futures-core only has one feature and that is alloc and it is on by default.

thomaseizinger avatar Sep 15 '22 03:09 thomaseizinger

While this is a nice simplification, I am not sure if it is necessarily a good idea. It is a little less clear exactly what features are enabled and which aren't, as we are relying on what the default features are, as opposed to writing them out ourself. What do you think?

Restioson avatar Sep 16 '22 07:09 Restioson

While this is a nice simplification, I am not sure if it is necessarily a good idea. It is a little less clear exactly what features are enabled and which aren't, as we are relying on what the default features are, as opposed to writing them out ourself. What do you think?

I would expect futures-core to always be extremely lightweight so the risk of having more code "slip in" is quite low I believe. Plus, I'd expect any changes to features to result in a minor version bump as they can be breaking.

Happy to change it back but I also like the simplicity of not needing to set features. I do see though how it can trigger the thought of "did we vet the default features of this lib?".

I wish default features would have never been introduced :(

thomaseizinger avatar Sep 19 '22 09:09 thomaseizinger

I would expect futures-core to always be extremely lightweight so the risk of having more code "slip in" is quite low I believe. Plus, I'd expect any changes to features to result in a minor version bump as they can be breaking.

Fair enough.

Happy to change it back but I also like the simplicity of not needing to set features. I do see though how it can trigger the thought of "did we vet the default features of this lib?".

Yea, this was my main point

Restioson avatar Sep 30 '22 14:09 Restioson

Happy to change it back but I also like the simplicity of not needing to set features. I do see though how it can trigger the thought of "did we vet the default features of this lib?".

Yea, this was my main point

Do you want me to add a comment?

thomaseizinger avatar Oct 02 '22 03:10 thomaseizinger

I think that's a good solution, thanks :)

Restioson avatar Oct 02 '22 21:10 Restioson

Thanks for adding the comment! Can be merged after conflicts are resolved

Restioson avatar Oct 11 '22 13:10 Restioson

Resolved! Sorry for the delay.

thomaseizinger avatar Oct 28 '22 01:10 thomaseizinger