Remove use of ARTRealtimePresence query in public API
#65
Conversation
See comment; the problem described there arose when Marat tried to implement a mock of Presence for #4.
WalkthroughThe changes involve updating the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
Sources/AblyChat/Presence.swift (2)
65-71: Excellent explanation and future-proofingThe detailed comments provide valuable context for the introduction of
PresenceQueryand demonstrate a thoughtful approach to addressing Swift concurrency issues. Creating an issue (#64) to revisit this implementation in the future is a great practice for tracking potential improvements.To enhance the comment further, consider adding a brief explanation of why
ARTRealtimePresenceQueryis notSendable, if known. This could provide additional context for future developers working on this code.
72-84: Approved: Well-structuredPresenceQueryimplementationThe
PresenceQuerystruct is well-implemented, addressing the compiler errors related toSendabletypes and providing a clear replacement forARTRealtimePresenceQuery. The properties cover the essential aspects of a presence query, and the use of default values in the initializer provides flexibility for API users.Consider making the initializer
publicinstead ofinternalto allow users of your library to create customPresenceQueryinstances more easily. This would enhance the usability of your API without compromising encapsulation.public init(limit: Int = 100, clientID: String? = nil, connectionID: String? = nil, waitForSync: Bool = true) { // ... (rest of the implementation remains the same) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- Sources/AblyChat/Presence.swift (2 hunks)
🔇 Additional comments (2)
Sources/AblyChat/Presence.swift (2)
8-8: Approved: Alignment with PR objectivesThe change from
ARTRealtimePresenceQuery?toPresenceQuery?in theget(params:)method signature aligns well with the PR objective of removing the use ofARTRealtimePresencequery in the public API. This modification likely improves type safety and makes the API more Swift-friendly.
Line range hint
1-84: Summary: Successful removal ofARTRealtimePresencequery from public APIThe changes in this file effectively achieve the PR objective of removing the use of
ARTRealtimePresencequery from the public API. The introduction of thePresenceQuerystruct as a replacement forARTRealtimePresenceQueryimproves type safety and addresses compiler errors related to non-sendable types in actor contexts.The modifications are well-documented, with detailed comments explaining the rationale behind the changes and considerations for future improvements. The implementation demonstrates a thoughtful approach to Swift concurrency issues and maintains the functionality of the presence query feature.
Overall, these changes enhance the API's usability and maintainability while setting the groundwork for potential future optimizations.
See comment; the problem described there arose when Marat tried to implement a mock of
Presencefor #4.Summary by CodeRabbit
New Features
PresenceQuerystruct to enhance presence management.getmethod in thePresenceprotocol for improved functionality.Bug Fixes
Presenceprotocol.