-
Notifications
You must be signed in to change notification settings - Fork 7
[ECO-5587, ECO-5589] Add missing documentation for public API #462
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis PR primarily adds missing documentation across many public API types, replaces swiftlint missing_docs disables with doc comments, and introduces a few new public APIs: two message update helpers, buffering-aware Presence.subscribe overloads, and explicit initializers for OccupancyEvent and RoomReactionEvent. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant Presence as Presence
participant SubscriptionSeq as SubscriptionAsyncSequence
Note over Caller,Presence: New public API: subscribe(bufferingPolicy:)
Caller->>Presence: subscribe(bufferingPolicy: policy)
alt policy handling
Presence->>SubscriptionSeq: create sequence with buffering policy
SubscriptionSeq-->>Caller: SubscriptionAsyncSequence<PresenceEvent>
end
sequenceDiagram
participant Updater as Caller
participant Message as Message
participant Event as ChatMessageEvent
Note over Updater,Message: Message.with(_ messageEvent:)
Updater->>Message: with(messageEvent)
alt event is Created
Message-->>Updater: throw ErrorInfo (rejected)
else ids mismatch
Message-->>Updater: throw ErrorInfo (identity mismatch)
else older event
Message-->>Updater: return original (no-op)
else newer event
Message->>Message: deep-copy reactions, apply update
Message-->>Updater: return updated Message
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/AblyChat/Connection.swift (1)
152-154: Fix time unit: TimeInterval is seconds, not milliseconds.
The doc states “milliseconds” butretryIn: TimeInterval?is in seconds. Update wording or change the type if milliseconds are intended.Apply this diff to fix the doc:
- /** - * The time in milliseconds that the client will wait before attempting to reconnect. - */ + /** + * The time in seconds (TimeInterval) that the client will wait before attempting to reconnect. + */
🧹 Nitpick comments (1)
Sources/AblyChat/JSONValue.swift (1)
103-142: Solid initializer docs; optional note on numeric precision.
Docs look good. Optionally add thatnumberstores IEEE‑754Double, which may round large integers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (21)
Sources/AblyChat/BufferingPolicy.swift(1 hunks)Sources/AblyChat/ChatClient.swift(5 hunks)Sources/AblyChat/Connection.swift(1 hunks)Sources/AblyChat/Events.swift(3 hunks)Sources/AblyChat/Headers.swift(2 hunks)Sources/AblyChat/JSONValue.swift(2 hunks)Sources/AblyChat/Logging.swift(2 hunks)Sources/AblyChat/Message.swift(1 hunks)Sources/AblyChat/MessageReaction.swift(2 hunks)Sources/AblyChat/MessageReactions.swift(1 hunks)Sources/AblyChat/Messages.swift(11 hunks)Sources/AblyChat/Occupancy.swift(2 hunks)Sources/AblyChat/PaginatedResult.swift(1 hunks)Sources/AblyChat/Presence.swift(6 hunks)Sources/AblyChat/Room.swift(2 hunks)Sources/AblyChat/RoomOptions.swift(5 hunks)Sources/AblyChat/RoomReactions.swift(2 hunks)Sources/AblyChat/Rooms.swift(2 hunks)Sources/AblyChat/Subscription.swift(1 hunks)Sources/AblyChat/SubscriptionAsyncSequence.swift(3 hunks)Sources/AblyChat/Typing.swift(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.swift: Use protocol-based design; expose SDK functionality via protocols and prefer associated types with opaque return types (some Protocol) instead of existentials (any Protocol)
Isolate all mutable state to the main actor; mark stateful objects with @mainactor
Public API must use typed throws with ErrorInfo; use InternalError internally and convert at the public API boundary
For public structs emitted by the API, provide an explicit public memberwise initializer
When using AsyncSequence operators in @mainactor contexts, mark operator closures as @sendable
Task, CheckedContinuation, and AsyncThrowingStream do not support typed errors; use Result and call .get() to surface typed errors
Do not use Dictionary.mapValues for typed throws; use ablyChat_mapValuesWithTypedThrow instead
When the compiler struggles with typed throws, explicitly declare the error type on do blocks (e.g., do throws(InternalError))
Specify error types in closures using the throws(ErrorType) syntax (e.g., try items.map { jsonValue throws(InternalError) in ... })
Mark any test-only APIs with testsOnly_ prefix and wrap them in #if DEBUG
Files:
Sources/AblyChat/RoomOptions.swiftSources/AblyChat/JSONValue.swiftSources/AblyChat/Events.swiftSources/AblyChat/Message.swiftSources/AblyChat/RoomReactions.swiftSources/AblyChat/Messages.swiftSources/AblyChat/Room.swiftSources/AblyChat/MessageReaction.swiftSources/AblyChat/Headers.swiftSources/AblyChat/PaginatedResult.swiftSources/AblyChat/Typing.swiftSources/AblyChat/Rooms.swiftSources/AblyChat/Occupancy.swiftSources/AblyChat/BufferingPolicy.swiftSources/AblyChat/Presence.swiftSources/AblyChat/Connection.swiftSources/AblyChat/Logging.swiftSources/AblyChat/SubscriptionAsyncSequence.swiftSources/AblyChat/Subscription.swiftSources/AblyChat/ChatClient.swiftSources/AblyChat/MessageReactions.swift
🧬 Code graph analysis (5)
Sources/AblyChat/Messages.swift (4)
Sources/AblyChat/Subscription.swift (1)
historyBeforeSubscribe(94-104)Example/AblyChatExample/Mocks/MockSubscriptionStorage.swift (1)
historyBeforeSubscribe(211-213)Sources/AblyChat/PaginatedResult.swift (1)
next(61-66)Sources/AblyChat/SubscriptionAsyncSequence.swift (3)
next(44-46)next(125-136)next(146-148)
Sources/AblyChat/Room.swift (5)
Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (1)
attach(425-439)Tests/AblyChatTests/Mocks/MockRoom.swift (1)
attach(47-49)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (1)
attach(94-102)Sources/AblyChat/RoomLifecycleManager.swift (2)
performAttachOperation(370-372)performAttachOperation(374-376)Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift (1)
performAttachOperation(25-31)
Sources/AblyChat/PaginatedResult.swift (4)
Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (2)
next(209-225)first(227-243)Tests/AblyChatTests/Mocks/MockHTTPPaginatedResponse.swift (2)
next(26-28)first(30-32)Example/AblyChatExample/Mocks/Misc.swift (2)
next(34-34)first(36-36)Tests/AblyChatTests/MessageSubscriptionResponseAsyncSequenceTests.swift (2)
next(14-14)first(16-16)
Sources/AblyChat/Typing.swift (2)
Sources/AblyChat/DefaultTyping.swift (1)
keystroke(117-132)Example/AblyChatExample/Mocks/MockClients.swift (1)
keystroke(419-427)
Sources/AblyChat/SubscriptionAsyncSequence.swift (1)
Sources/AblyChat/Messages.swift (1)
next(491-493)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Xcode, tvOS (Xcode 26.0)
- GitHub Check: Example app, tvOS (Xcode 26.0)
- GitHub Check: Example app, iOS (Xcode 26.0)
- GitHub Check: Xcode,
releaseconfiguration, iOS (Xcode 26.0) - GitHub Check: Xcode,
releaseconfiguration, macOS (Xcode 26.0) - GitHub Check: Xcode,
releaseconfiguration, tvOS (Xcode 26.0) - GitHub Check: Example app, macOS (Xcode 26.0)
- GitHub Check: Xcode, iOS (Xcode 26.0)
- GitHub Check: Xcode, macOS (Xcode 26.0)
- GitHub Check: SPM,
releaseconfiguration (Xcode 26.0) - GitHub Check: SPM (Xcode 26.0)
- GitHub Check: check-documentation
🔇 Additional comments (53)
Sources/AblyChat/Subscription.swift (1)
37-38: LGTM: Doc for HistoryResult associated type is clear and accurate.
No action needed.Sources/AblyChat/Connection.swift (1)
8-9: LGTM: StatusSubscription associated type docs.
Reads well and aligns with the protocol.Sources/AblyChat/JSONValue.swift (1)
32-43: LGTM: Enum case documentation is clear and consistent.
No changes needed.Sources/AblyChat/MessageReaction.swift (2)
140-147: LGTM: ClientIDList initializer docs.
Clear purpose and mock-usage note. No changes needed.
181-190: LGTM: ClientIDCounts initializer docs.
Consistent and helpful. No changes needed.Sources/AblyChat/Events.swift (3)
11-18: LGTM: ChatMessageAction case docs.
Concise and accurate.
88-91: LGTM: TypingEventType case docs.
Clear semantics.
105-106: LGTM: TypingSetEventType documentation.
Looks good.Sources/AblyChat/Logging.swift (1)
3-5: LGTM! Clear and concise documentation.The documentation added for
LogHandlerandLogLevelcases is well-written and provides appropriate context for users of the public API.Also applies to: 39-48
Sources/AblyChat/PaginatedResult.swift (1)
3-30: LGTM! Comprehensive protocol documentation.The documentation clearly describes the
PaginatedResultprotocol and all its members, making the pagination API easy to understand and use.Sources/AblyChat/RoomReactions.swift (2)
10-10: LGTM! Documentation additions are clear.The documentation for the associatedtype and event-related types appropriately describes their purpose in the public API.
Also applies to: 112-116, 118-123
128-131: Public initializer added per coding guidelines.The explicit public memberwise initializer follows the coding guideline for public structs. The default value for
typemakes sense sinceRoomReactionEventTypecurrently only has one case (.reaction).Sources/AblyChat/SubscriptionAsyncSequence.swift (2)
62-67: LGTM! Improved mock initializer documentation.The concise documentation clearly explains the purpose of the mock initializer and appropriately warns that passing a throwing
AsyncSequenceis a programmer error.
119-119: LGTM! Standard AsyncSequence documentation.The documentation for
AsyncIteratorand its methods follows Swift conventions and clearly describes their behavior.Also applies to: 145-146, 151-151
Sources/AblyChat/Headers.swift (1)
5-12: LGTM! Clear documentation for HeadersValue.The documentation for enum cases and literal conformance initializers appropriately describes their purpose and usage.
Also applies to: 54-55, 61-62, 68-69, 75-76
Sources/AblyChat/BufferingPolicy.swift (1)
6-11: LGTM! Clear BufferingPolicy documentation.The documentation concisely describes each buffering strategy, making it easy for users to choose the appropriate policy.
Sources/AblyChat/MessageReactions.swift (1)
10-10: LGTM! Consistent associatedtype documentation.The documentation for the
Subscriptionassociatedtype follows the same pattern used in other protocols throughout the PR.Sources/AblyChat/RoomOptions.swift (1)
27-35: LGTM! Comprehensive initializer documentation.The documentation for all option struct initializers is well-structured and consistent, clearly describing the purpose and parameters of each initializer.
Also applies to: 57-62, 92-98, 121-126, 145-150
Sources/AblyChat/Messages.swift (10)
11-16: LGTM! Clear documentation for associated types.The documentation concisely describes each associated type's purpose, improving API discoverability.
118-118: LGTM! Extension documentation added.The documentation clearly describes the extension's purpose.
197-209: LGTM! Comprehensive initializer documentation.The documentation clearly describes all parameters and follows the coding guideline for explicit public memberwise initializers.
239-250: LGTM! Initializer documentation added.The documentation is clear and follows Swift documentation conventions.
256-263: LGTM! Enum case documentation added.Clear and concise descriptions for both enum cases.
300-314: LGTM! Comprehensive initializer documentation.The documentation clearly describes all parameters.
346-358: LGTM! Initializer documentation added.Clear documentation for all parameters.
403-408: LGTM! Enum case documentation added.Clear descriptions for all event types.
411-416: LGTM! Struct and property documentation added.Clear and concise documentation.
443-444: LGTM! AsyncSequence documentation comprehensive.All components of the AsyncSequence implementation are clearly documented, including the mock initializer for testing.
Also applies to: 460-463, 477-478, 482-483, 490-491, 496-497
Sources/AblyChat/Typing.swift (5)
11-11: LGTM! Associated type documentation added.Clear description of the subscription type.
33-46: LGTM! Comprehensive behavioral documentation.The expanded documentation clearly explains throttling, serialization guarantees, and no-op behavior for intermediate calls. This provides essential information for developers using the API.
50-62: LGTM! Consistent and comprehensive documentation.The documentation mirrors the keystroke() method's structure and clearly explains the serialization guarantees and behavior, ensuring consistency across the API.
99-114: LGTM! Struct and property documentation added.Clear descriptions for all properties in TypingSetEvent.
126-131: LGTM! Nested struct documentation added.Clear descriptions for the Change struct properties.
Sources/AblyChat/Rooms.swift (2)
8-11: LGTM! Associated type documentation added.Clear descriptions for both associated types.
57-60: LGTM! Extension and convenience method documented.Clear descriptions for both the extension and the convenience method.
Sources/AblyChat/ChatClient.swift (5)
3-15: LGTM! Protocol and associated type documentation added.The documentation clearly explains the protocol's purpose for testing/mocking and describes all associated types.
75-113: LGTM! Property documentation comprehensive.All properties are clearly documented with consistent formatting.
118-125: LGTM! Enhanced initializer documentation.The Important note about clientId requirement is valuable information for developers.
154-162: LGTM! Property documentation with important usage note.The Important note about token-based authentication is valuable for developers.
186-192: LGTM! Initializer documentation added.Clear parameter documentation for ChatClientOptions.
Sources/AblyChat/Message.swift (3)
212-212: LGTM! Extension documentation added.Clear description of the extension's purpose.
214-233: LGTM! New public API method with proper validation.The method correctly:
- Uses typed throws with ErrorInfo (per coding guidelines)
- Validates message serial matches event serial before applying changes
- Follows immutable pattern by returning a new instance
- Provides clear documentation
235-267: LGTM! New public API method with comprehensive validation.The method correctly:
- Uses typed throws with ErrorInfo (per coding guidelines)
- Rejects created events (which cannot be applied to existing messages)
- Validates message serial matches
- Uses lexicographic comparison per spec (CHA-M10e)
- Returns unchanged message if event is older (optimization)
- Preserves reactions when creating updated message
Sources/AblyChat/Room.swift (2)
8-23: LGTM! Comprehensive associated type documentation.All seven associated types are clearly documented.
376-377: LGTM! Method documentation references added.The "See" references to protocol methods are a good practice for implementation documentation.
Also applies to: 381-382
Sources/AblyChat/Occupancy.swift (3)
11-11: LGTM! Associated type documentation added.Clear description of the subscription type.
101-104: LGTM! Enum and case documentation added.Clear descriptions for the occupancy event type.
107-120: LGTM! Explicit public initializer added per coding guidelines.The explicit memberwise initializer follows the coding guideline for public structs and is properly documented for testing/mocking purposes.
Sources/AblyChat/Presence.swift (5)
3-3: LGTM! Typealias and associated type documentation added.Clear descriptions for both types.
Also applies to: 14-14
114-146: LGTM! New AsyncSequence convenience methods follow established patterns.The new subscribe methods:
- Follow the same pattern as Messages.swift
- Properly create and manage SubscriptionAsyncSequence
- Include termination handlers for cleanup
- Are comprehensively documented with important usage notes
149-153: LGTM! Enhanced struct documentation with important behavioral details.The documentation clearly explains the uniqueness semantics of presence members and the multi-device scenario, which is valuable for developers.
185-188: LGTM! Property documentation added.Clear description of the updatedAt property.
281-293: LGTM! Comprehensive initializer documentation.Clear parameter documentation for all PresenceParams initializer parameters.
| case deleted | ||
| } | ||
|
|
||
| /// Event emitted by message subscriptions, containing the type and the message. |
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 this be in the previous commit given that it's an update?
Closes #377, #388
Summary by CodeRabbit
New Features
Documentation