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

[FIXED] MQTT PUBREL header incompatibility #4616

Merged
merged 2 commits into from Oct 5, 2023
Merged

Conversation

levb
Copy link
Contributor

@levb levb commented Oct 2, 2023

https://hivemq.github.io/mqtt-cli/docs/test/ pointed out the incompatibility.

@levb
Copy link
Contributor Author

levb commented Oct 2, 2023

@wallyqs Please advice where can I add running this test, mqtt test -V 3 -h {NATS_SERVER}

Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

LGTM

@wallyqs
Copy link
Member

wallyqs commented Oct 2, 2023

@levb I'm thinking that maybe you can add a test under mqtt_test.go that starts the server and checks whether mqtt is installed and if not installed it skips the test, then in README-MQTT add the instructions about the dependency needed to run the compatibility test and we use a different worker for test (not sure about adding installing mqtt-cli as part of the travis run since it is Java).

@levb levb marked this pull request as ready for review October 3, 2023 12:52
@levb levb requested a review from a team as a code owner October 3, 2023 12:52
@levb levb requested a review from wallyqs October 3, 2023 12:52
@levb
Copy link
Contributor Author

levb commented Oct 3, 2023

@wallyqs please check the new test/action

Copy link
Member

@wallyqs wallyqs left a comment

Choose a reason for hiding this comment

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

LGTM

@levb levb merged commit beee6fc into main Oct 5, 2023
4 checks passed
@levb levb deleted the lev-mqtt-pubrel-compat branch October 5, 2023 15:07
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

3 participants