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

Enable websocket feature by default #734

Merged
merged 8 commits into from
Oct 21, 2023
Merged

Enable websocket feature by default #734

merged 8 commits into from
Oct 21, 2023

Conversation

swanandx
Copy link
Member

This PR does following:

  • enable websocket feature by default
  • remove old "websockets" feature
  • log warning if ws config is getting ignored

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • Formatted with cargo fmt
  • Make an entry to CHANGELOG.md if it's relevant to the users of the library. If it's not relevant mention why.

@swanandx swanandx requested a review from h3nill October 20, 2023 09:44
Copy link

@h3nill h3nill left a comment

Choose a reason for hiding this comment

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

changes LGTM, but i rememver last time having a feature with same name between rumqttc and rumqttd caused some problem. Can you check if its still the case?

more context here: #633

@swanandx
Copy link
Member Author

swanandx commented Oct 20, 2023

I ran cargo r --bin rumqttd --features websocket from root of repo and it works without any issues!

IIRC, the issue was it enabled websocket feature for both rumqttc and rumqttd and some dependencies caused conflicts.and to specify depency for a single package, you suggested this.

PS: tested cargo r --bin rumqttd as well to verify it is enabled by default.

@swanandx swanandx merged commit c6d6d93 into main Oct 21, 2023
3 checks passed
@swanandx swanandx deleted the ws-feature branch October 21, 2023 09:34
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.

None yet

2 participants