Skip to content
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

featue leak fixed, test added #604

Merged
merged 8 commits into from
Nov 29, 2023
Merged

featue leak fixed, test added #604

merged 8 commits into from
Nov 29, 2023

Conversation

milyin
Copy link
Contributor

@milyin milyin commented Nov 28, 2023

there was a leak of zenoh's default features through zenoh-ext. Fixed it, added test to zenohd to control this in future

@Mallets
Copy link
Member

Mallets commented Nov 28, 2023

This PR looks good to me, however I believe it may break our plugins and bindings in cascade without additional PRs.
E.g., zenoh-c needs to explicitly add default-features = false when including zenoh and add default = ["zenoh/default"] for the crate's default features.

@milyin
Copy link
Contributor Author

milyin commented Nov 28, 2023

This PR looks good to me, however I believe it may break our plugins and bindings in cascade without additional PRs. E.g., zenoh-c needs to explicitly add default-features = false when including zenoh and add default = ["zenoh/default"] for the crate's default features.

There is no break for dependent crates/projects. zenoh-c is not required to add "default-features = false", it just can add this parameter now and be sure that no unnecessary features leak out through another crate.
I.e. it's still possible to include zenoh with or without default features, as before. The only difference that now including zenoh with default-features = false really works

@Mallets Mallets merged commit 7fa7d6c into master Nov 29, 2023
12 of 14 checks passed
@Mallets Mallets deleted the feature_cleanup2 branch November 29, 2023 08:27
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