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

Update zbus to v5 #84

Merged
merged 1 commit into from
Feb 13, 2025
Merged

Update zbus to v5 #84

merged 1 commit into from
Feb 13, 2025

Conversation

liff
Copy link
Contributor

@liff liff commented Nov 1, 2024

zbus 5.0.0 release

Some fairly minor changes needed.

In zbus v5 the blocking API is feature-gated, so I wonder if the blocking secret service API should be similarly behind a feature?

Copy link
Collaborator

@complexspaces complexspaces left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you wouldn't mind rebasing this on master, this looks good to go with the current changes 👍.

In zbus v5 the blocking API is feature-gated, so I wonder if the blocking secret service API should be similarly behind a feature?

According to the zbus docs the blocking-api feature is enabled by default. I wouldn't mind adding the feature to parity match but I'm not entirely sure how it would interact with the existing features we have today. IIUC you would still need to select a rt- feature to use the blocking API, so it might just be additive and not create another dimension. Does that sound correct to you as well?

Long story short I'm in favor of the feature flag for consistency as long as it doesn't make the rt-tokio-crypto-rust problem worse.

@liff
Copy link
Contributor Author

liff commented Feb 11, 2025

Rebased.

IIUC you would still need to select a rt- feature to use the blocking API, so it might just be additive and not create another dimension

I don’t think there’d be another dimension. Disabling the blocking-api feature would basically just omit the secret_service::blocking module, if I remember correctly. It’s not something that needs dealing with, but could probably be done if there turn out to be useful aspects to it.

Copy link
Collaborator

@complexspaces complexspaces left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks for helping clarify. Since it doesn't add that much code to the build and doesn't bring in more dependencies having it enabled I'm OK leaving it be for now and seeing if the feature request comes up.

@complexspaces complexspaces merged commit 3a0441a into hwchen:master Feb 13, 2025
13 checks passed
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