Add support for channel groups #32
Conversation
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
jguz-pubnub
force-pushed
the
feat/channel-groups
branch
2 times, most recently
from
96546bc to
e8191d1
Compare
jguz-pubnub
force-pushed
the
feat/channel-groups
branch
from
e8191d1 to
aec9fd6
Compare
| /// - completion: The async `Result` of the method call | ||
| /// - **Success**: A `Void` indicating a success | ||
| /// - **Failure**: An `Error` describing the failure | ||
| func add(channels: [ChatType.ChatChannelType], completion: ((Swift.Result<Void, Error>) -> Void)?) |
There was a problem hiding this comment.
What do you think about defining
typealias OperationResult = Swift.Result<Void, Error>
and use it in several places?
There was a problem hiding this comment.
It's clearer to leave the Result type as-is, which also aligns with the convention used in the core SDK. In this context, introducing typealiases leads to obscure the actual return type, forcing the caller to dig deeper to understand what's being returned
Sources/Chat.swift
Outdated
| @@ -203,7 +204,7 @@ public protocol Chat: AnyObject { | |||
| /// - filter: Expression used to filter the results. Returns only these channels whose properties satisfy the given expression are returned | |||
There was a problem hiding this comment.
Instead:
Returns only these channels whose properties satisfy the given expression are returned
shouldn't be:
Returns only the channels whose properties satisfy the given expression.
?
There was a problem hiding this comment.
Yup, makes sense. Something is off with the current comment. I will change it to what you suggest
| /// Returns a paginated list of all existing channels in a given ``ChannelGroup``. | ||
| /// | ||
| /// - Parameters: | ||
| /// - filter: Expression used to filter the results. Returns only these channels whose properties satisfy the given expression are returned |
There was a problem hiding this comment.
Instead:
Returns only these channels whose properties satisfy the given expression are returned
shouldn't be:
Returns only the channels whose properties satisfy the given expression.
?
Sources/Entities/ChannelGroup.swift
Outdated
| /// Returns a paginated list of all existing channels in a given ``ChannelGroup``. | ||
| /// | ||
| /// - Parameters: | ||
| /// - filter: Expression used to filter the results. Returns only these channels whose properties satisfy the given expression are returned |
There was a problem hiding this comment.
Instead:
Returns only these channels whose properties satisfy the given expression are returned
shouldn't be:
Returns only the channels whose properties satisfy the given expression.
?
| ) | ||
| } | ||
|
|
||
| public func connect(callback: @escaping (MessageImpl) -> Void) -> AutoCloseable { |
There was a problem hiding this comment.
Is this ok that this line is not consistent with ChannelGroup protocol:
func connect(callback: @escaping (ChatType.ChatMessageType) -> Void) -> AutoCloseable
?
There was a problem hiding this comment.
Yes, it's consistent. The escaping keyword here is required and means the closure can be called after the function it was passed to has returned
|
@pubnub-release-bot release as 0.31.0 |
|
🚀 Release successfully completed 🚀 |
4 participants
feat(channel-groups): add support for channel groups