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

Split the PubSub sample down into smaller samples #233

Merged
merged 12 commits into from
Apr 1, 2022

Conversation

TwistedTwigleg
Copy link
Contributor

@TwistedTwigleg TwistedTwigleg commented Mar 22, 2022

Description of changes:

Splits the pub-sub sample down into multiple smaller samples that connect using a single method, rather than having one large pub-sub sample that can connect with multiple methods. This makes it easier for users to see how to connect using each method.

Also adds helper functions to the commandLineUtils function for making a MQTT connection without needing to manually set it up, removing the MQTT connection code duplication across samples and instead allowing them to focus on showing the key feature of the sample.

Finally, this PR also refactors the samples a little to make them a bit more compact and easier to follow.

Edit: This PR has an issue with references not being fully released, causing the samples that use the MQTT builder to pause for about a minute. This issue should be fixed by the following PR: awslabs/aws-crt-java#459
(Will need to be tested again once the PR is merged)


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

* Added PubSub sample back to codebuild tests
* Fixed typos and little code mistakes
* Simplified connection samples to use Utility functions in CommandLineUtils
* Added function for the connection/disconnection code that is the same across all connection samples and added it to CommandLineUtils for reuse across samples
@TwistedTwigleg
Copy link
Contributor Author

Thanks for the review! I adjusted the code accordingly 👍

@graebm
Copy link
Contributor

graebm commented Mar 24, 2022

You'll need to update the new WindowsCertPubSub sample too (it just landed in main)

@TwistedTwigleg
Copy link
Contributor Author

Awesome, will do!

@TwistedTwigleg
Copy link
Contributor Author

Updated the WindowsCertPubSub sample so it is a connect sample and made (really minor) changes so it is similar to the other connect samples 👍

@TwistedTwigleg
Copy link
Contributor Author

Thanks for the review! Merging into main...

@TwistedTwigleg TwistedTwigleg merged commit 79e839b into main Apr 1, 2022
@TwistedTwigleg TwistedTwigleg deleted the PubSubSampleSplit branch April 1, 2022 19: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.

4 participants