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.

Sign up for GitHub

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

Jump to bottom

feat: add onConnectionLost and onConnectionRestored callback functions #342

Merged
merged 5 commits into from
Dec 15, 2023
Merged

feat: add onConnectionLost and onConnectionRestored callback functions #342

merged 5 commits into from
Dec 15, 2023

Conversation

rishtigupta
Copy link
Contributor

@rishtigupta rishtigupta commented Dec 14, 2023 •
edited
Loading

PR Description:

  • add onConnectionLost and onConnectionRestored callback functions
  • make them default so that the user is not compelled to implement the functions

Testing

Manual Testing

  • Run TopicExample in the examples/topics directory
  • Switch off the wifi in the middle of messages being published
  • Wait for the server to throw an exception (UNAVAILABLE) exception
  • Switch on the wifi
  • Verify logging statements in the default onConnectionLost and default onConnectionRestored are invoked

Unit Testing

  • SubscriptionWrapperTest in unit tests.

@rishtigupta rishtigupta marked this pull request as ready for review December 15, 2023 01:12
cprice404
cprice404 reviewed Dec 15, 2023
View reviewed changes
Copy link
Contributor

@cprice404 cprice404 left a comment

Choose a reason for hiding this comment

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

lgtm! just one suggestion to make it clear why that method changed to public, then I'm 👍

cprice404
cprice404 approved these changes Dec 15, 2023
View reviewed changes
Copy link
Contributor

@cprice404 cprice404 left a comment

Choose a reason for hiding this comment

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

heart

@rishtigupta rishtigupta merged commit 51ca3b4 into main Dec 15, 2023
5 checks passed
@rishtigupta rishtigupta deleted the feat/add-callback-methods branch December 15, 2023 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cprice404 cprice404 cprice404 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rishtigupta @cprice404