Skip to content

Conversation

@huntc
Copy link
Contributor

@huntc huntc commented Jan 2, 2025

Defaults shouldn't be enabled for transitive lib dependencies. They can create additive issues downstream e.g. when using with WASM.

@huntc huntc requested a review from arnolddevos January 2, 2025 03:38
Defaults shouldn't be enabled for transitive lib dependencies. They can create additive issues downstream e.g. when using with WASM.
@arnolddevos
Copy link
Collaborator

Does this change have any practical effect? Changing workspace dependencies only matters if they are used in the workspace somewhere, right? Changing dev-dependencies does not affect the build.

@huntc
Copy link
Contributor Author

huntc commented Jan 3, 2025

Does this change have any practical effect? Changing workspace dependencies only matters if they are used in the workspace somewhere, right? Changing dev-dependencies does not affect the build.

You're correct. However, I think the default features disablement in the workspace is more correct anyhow. Also, edfsm-kvstore was previously depending on evfsm-machine with default features, so it did matter then.

However, the more important change here is the removal of a dependency on edfsm-machine by the edfsm-kvstore. Given that removal, we no longer transitiviely enable the default features of edfsm-machine when depending on edfsm-kvstore. I have just re-proved this with my project that is WASM and therefore doesn't cope with edfsm-machine and default features.

@arnolddevos
Copy link
Collaborator

removal of a dependency on edfsm-machine by the edfsm-kvstore

But you removed the dev dependency and left the build dependency. The build dependency is optional but required by the tokio feature (see async_query.rs) Also the tokio feature remains a default feature in kv-store (which I am OK with).

@arnolddevos
Copy link
Collaborator

I don't think any changes are needed here. The tokio dep in edfsm-machine is on feature "sync" only and is require. The machine dep in edfsm-kv-store is optional but necessary when compiling async_query. The tokio dep is again on "sync" only.

huntc added 2 commits January 4, 2025 11:40
Also, removes the streambed feature by default
@huntc huntc merged commit 6eb7093 into titanclass:main Jan 4, 2025
1 check passed
@huntc huntc deleted the avoid-features branch January 4, 2025 00:49
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