-
Notifications
You must be signed in to change notification settings - Fork 8
Add missing implementation for ChannelGroup entity (Apple and JS platforms) #355
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
override fun subscription(options: SubscriptionOptions): Subscription { | ||
// TODO: Add support for handling SubscriptionOptions | ||
return SubscriptionImpl(objCSubscription = KMPSubscription(entity = channelGroup)) | ||
val presenceOptions = options.allOptions.filterIsInstance<ReceivePresenceEventsImpl>() |
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 need to add tests?
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.
I see there are tests for a channel/channel group entity in this repository:
However, by default, they are only executed on the jvm target. We would need to replicate the same effort here as was done in the KMP chat project
override val name: String | ||
get() = jsChannelGroup.name | ||
|
||
override fun subscription(options: SubscriptionOptions): Subscription { // todo use options |
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.
Is todo comment up to date ?
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.
I think so, because there's no support for FilterImpl
options
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.
I see. I was misled because I saw options being used in code below. Maybe todo comment could be modified with more precise info?
override val name: String | ||
get() = jsChannelGroup.name | ||
|
||
override fun subscription(options: SubscriptionOptions): Subscription { // todo use options |
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.
I see. I was misled because I saw options being used in code below. Maybe todo comment could be modified with more precise info?
@pubnub-release-bot release kotlin as v10.5.3 |
🛑 Build failed. Please check release build for details 🛑 |
fix: Internal fixes