Skip to content

Add L-2 communication - #27

Open
dddj698 wants to merge 3 commits into
eclipse-uprotocol:mainfrom
dddj698:up1.5.8-L2
Open

Add L-2 communication #27
dddj698 wants to merge 3 commits into
eclipse-uprotocol:mainfrom
dddj698:up1.5.8-L2

Conversation

@dddj698

@dddj698 dddj698 commented Jun 27, 2024

Copy link
Copy Markdown
Contributor

Add L-2 communication with Examples.

@dddj698

dddj698 commented Jun 27, 2024

Copy link
Copy Markdown
Contributor Author

Rebase and Merge after #26

@dddj698
dddj698 requested a review from stevenhartley June 27, 2024 15:17
@github-actions

Copy link
Copy Markdown

Code coverage report is ready! 📈

@stevenhartley stevenhartley left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Small issue in the InMemorySubscriber only

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I have moved this to L3 folder for USubscription service client implementation, you might want to do the same after this is merged

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

will do in following PR

* @param options The call options for the subscription.
* @return Returns [UStatus] with the result from the unsubscribe request.
*/
override suspend fun unsubscribe(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You're not removing the SubscriptionChangeHandler that might have been registered in the subscribe() API

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it is removed in line 135 if invokeMethod success

@dddj698
dddj698 requested a review from stevenhartley July 26, 2024 19:54
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