-
Notifications
You must be signed in to change notification settings - Fork 0
Rework subscription loop to use event engine #46
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
} | ||
|
||
enum pubnub_res AddListenerResult = pubnub_subscribe_add_subscription_listener(Subscription, PBSL_LISTENER_ON_MESSAGE, Callback, PubnubSubsystem); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we let developer to select kind of the listener?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, for default subscription loop we only use subscription listener. By this PR we replace old subscription loop with Event Engine. This is not about exposing listeners to the user.
} | ||
|
||
bool UPubnubUtilities::EEAddListenerAndSubscribe(pubnub_subscription_t* Subscription, pubnub_subscribe_message_callback_t Callback, UPubnubSubsystem* PubnubSubsystem) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why prefix EE
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For "Event Engine", these are internal functions not exposed to the user, so as they are directly connected to Event Engine I preferred to mark them like this.
… all requested data from server bug.
modify(subscription): Rework subscription loop to use event engine
remove(c-core): Remove not needed c-core files from Include folder
change(params): create dedicated structures for
Sort
andInclude
paramsRework
GetMemberships
,GetChannelMembers
,GetAllUsersMetadataand
andGetAllChannelsMetadata
to have dedicated structures for Include and Sort fields. Note: this is BREAKING CHANGE.change(deprecate): Mark _JSON functions as deprecated
fix(fetchHistory): Fix bugs in parsing
FetchHistory
response.fix(responses): Fix bug that not all requested data was included in functions response when TotalCount was set.