chore: turn on explicitApi for kotlin
#114
Conversation
WalkthroughThis pull request introduces a Kotlin explicit API configuration in the Gradle build script and updates the visibility modifiers for many exported entities throughout the codebase. Most interfaces, classes, enums, data classes, type aliases, and methods in the chat module have been changed from package-private or internal to public. A single constant has been made private, and one utility extension property is now explicitly marked as internal. These modifications expand the API surface without altering the underlying functionality. Changes
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 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 (
|
ttypic
force-pushed
the
turn-on-explicit-api
branch
from
163e5ae to
870168c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
chat-android/src/main/java/com/ably/chat/Occupancy.kt (1)
67-68: 🛠️ Refactor suggestionVisibility modifiers missing on data class properties
The properties
connectionsandpresenceMembersin theOccupancyEventdata class are missing explicit visibility modifiers. WithexplicitApimode enabled, these should also have explicit visibility modifiers./** * The number of connections to the chat room. */ -val connections: Int, +public val connections: Int, /** * The number of presence members in the chat room - members who have entered presence. */ -val presenceMembers: Int, +public val presenceMembers: Int,Also applies to: 71-72
🧹 Nitpick comments (14)
chat-android/src/main/java/com/ably/chat/Logger.kt (1)
1-140: Consider adding public visibility to properties of LogContextWhile you've correctly marked the
LogContextclass as public, the properties inside it (tag, staticContext, dynamicContext) don't have explicit visibility modifiers. When usingexplicitApimode, it's generally good practice to make all public API members explicitly public.Consider making the properties in
LogContextexplicitly public:public data class LogContext( - val tag: String, - val staticContext: Map<String, String> = mapOf(), - val dynamicContext: Map<String, () -> String> = mapOf(), + public val tag: String, + public val staticContext: Map<String, String> = mapOf(), + public val dynamicContext: Map<String, () -> String> = mapOf(), )This would make all public API elements explicitly visible, which is the goal of enabling
explicitApimode.🧰 Tools
🪛 GitHub Actions: Check
[warning] 8-8: LogHandler is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 9-9: log is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 12-12: LogContext is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/RoomOptions.kt (3)
9-9: Redundant public modifierThis class is explicitly marked as public, but public is the default visibility for classes in Kotlin. While redundant from a language perspective, this is expected behavior when enabling
explicitApi().Consider suppressing the RedundantVisibilityModifierRule warning by adding a suppression annotation if these warnings are being flagged in the CI pipeline:
@Suppress("RedundantVisibilityModifier") public data class RoomOptions(🧰 Tools
🪛 GitHub Actions: Check
[warning] 9-9: RoomOptions is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
35-39: Redundant public modifiers in companion object and propertyThe companion object and
defaultproperty are explicitly marked as public, but they would be public by default. These redundant modifiers are expected when usingexplicitApi().If desired, you can suppress these warnings:
@Suppress("RedundantVisibilityModifier") public companion object { /** * Supports all room options with default values */ public val default: RoomOptions = RoomOptions(🧰 Tools
🪛 GitHub Actions: Check
[warning] 39-39: default is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
51-51: Redundant public modifiers on data classesAll these classes are explicitly marked as public, which is redundant as public is the default visibility in Kotlin. These explicit modifiers are expected when using
explicitApi().Consider adding a file-level suppression to handle all redundant visibility modifiers at once:
@file:Suppress("RedundantVisibilityModifier")Also applies to: 72-72, 87-87, 98-98
🧰 Tools
🪛 GitHub Actions: Check
[warning] 51-51: PresenceOptions is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/RoomReactions.kt (2)
18-18: Redundant public modifiers on interfacesThe
RoomReactionsandListenerinterfaces are explicitly marked as public, which is redundant since public is the default visibility for interfaces in Kotlin. This is expected when enablingexplicitApi().Consider adding a file-level suppression for these warnings:
@file:Suppress("RedundantVisibilityModifier")Also applies to: 54-54
🧰 Tools
🪛 GitHub Actions: Check
[warning] 18-18: RoomReactions is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
25-25: Redundant public modifiers on interface membersThe
channelproperty,sendandsubscribemethods, andonReactionmethod are explicitly marked as public, which is redundant since members of interfaces are public by default. This is expected behavior when enablingexplicitApi().These redundant modifiers could be suppressed with a file-level annotation, but they're useful for API clarity with
explicitApi()enabled.Also applies to: 41-41, 49-49, 59-59
🧰 Tools
🪛 GitHub Actions: Check
[warning] 25-25: channel is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Messages.kt (3)
23-23: Redundant public modifiers on interfacesThe
Messages,Listener, andMessagesSubscriptioninterfaces are explicitly marked as public, which is redundant as public is the default visibility for interfaces in Kotlin. These redundant modifiers are expected when enablingexplicitApi().Consider adding a file-level suppression if these warnings are causing issues in the pipeline:
@file:Suppress("RedundantVisibilityModifier")Also applies to: 111-111, 267-267
29-29: Redundant public modifiers on interface membersThese members are explicitly marked as public, which is redundant since interface members are public by default. This is expected behavior with
explicitApi()enabled.Also applies to: 36-36, 48-53, 71-71, 86-86, 106-106, 116-116, 273-273
86-89: Fix parameter list wrapping for the update methodThere's a warning in the pipeline about parameter list wrapping. The method signature should be reformatted according to the project's coding standards.
Apply this formatting to fix the parameter list wrapping issue:
-public suspend fun update(updatedMessage: Message, operationDescription: String? = null, operationMetadata: OperationMetadata? = null): Message +public suspend fun update( + updatedMessage: Message, + operationDescription: String? = null, + operationMetadata: OperationMetadata? = null, +): Message🧰 Tools
🪛 GitHub Actions: Check
[warning] 86-89: Parameter should start on a newline [ParameterListWrapping]
[warning] 89-89: Missing newline before ')' [ParameterListWrapping]
[warning] 89-89: Missing trailing comma before ')' [TrailingCommaOnDeclarationSite]
chat-android/src/main/java/com/ably/chat/Connection.kt (3)
24-24: Redundant public modifier on enum with public propertyThe
ConnectionStatusenum and itsstateNameproperty are explicitly marked as public, which is redundant as they would be public by default. This is expected behavior when enablingexplicitApi().Consider suppressing the redundant visibility modifier warning if needed:
@Suppress("RedundantVisibilityModifier") public enum class ConnectionStatus(public val stateName: String) {🧰 Tools
🪛 GitHub Actions: Check
[warning] 24-24: ConnectionStatus is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
59-59: Redundant public modifiers on class and interfacesThe
ConnectionStatusChangedata class,Connectioninterface, andListenerinterface are explicitly marked as public, which is redundant since these would be public by default. This is expected when usingexplicitApi().Also applies to: 85-85, 106-106
🧰 Tools
🪛 GitHub Actions: Check
[warning] 59-59: ConnectionStatusChange is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
89-89: Redundant public modifiers on interface membersThe properties
statusanderror, and methodsonStatusChangeandconnectionStatusChangedare explicitly marked as public, which is redundant as interface members are public by default. This is expected behavior withexplicitApi()enabled.Also applies to: 94-94, 101-101, 111-111
🧰 Tools
🪛 GitHub Actions: Check
[warning] 89-89: status is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Occupancy.kt (2)
24-24: Consider adding API stability annotationSince you're now defining an explicit public API, consider adding an API stability annotation (like
@ExperimentalApior@StableApi) to clearly communicate the stability status of this API to consumers.🧰 Tools
🪛 GitHub Actions: Check
[warning] 24-24: Occupancy is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
67-72: Add code example in KDoc for better API documentationNow that you're defining an explicit public API with
explicitApi, consider enhancing the KDoc for the public data class properties with usage examples to make the API more approachable for consumers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
chat-android/build.gradle.kts(1 hunks)chat-android/src/main/java/com/ably/chat/ChatClient.kt(1 hunks)chat-android/src/main/java/com/ably/chat/ChatClientOptions.kt(1 hunks)chat-android/src/main/java/com/ably/chat/Connection.kt(3 hunks)chat-android/src/main/java/com/ably/chat/Discontinuities.kt(1 hunks)chat-android/src/main/java/com/ably/chat/ErrorCodes.kt(2 hunks)chat-android/src/main/java/com/ably/chat/EventTypes.kt(5 hunks)chat-android/src/main/java/com/ably/chat/Headers.kt(1 hunks)chat-android/src/main/java/com/ably/chat/LogLevel.kt(1 hunks)chat-android/src/main/java/com/ably/chat/Logger.kt(1 hunks)chat-android/src/main/java/com/ably/chat/Message.kt(1 hunks)chat-android/src/main/java/com/ably/chat/Messages.kt(7 hunks)chat-android/src/main/java/com/ably/chat/Metadata.kt(1 hunks)chat-android/src/main/java/com/ably/chat/Occupancy.kt(3 hunks)chat-android/src/main/java/com/ably/chat/OperationMetadata.kt(1 hunks)chat-android/src/main/java/com/ably/chat/OrderBy.kt(1 hunks)chat-android/src/main/java/com/ably/chat/PaginatedResult.kt(1 hunks)chat-android/src/main/java/com/ably/chat/Presence.kt(3 hunks)chat-android/src/main/java/com/ably/chat/Reaction.kt(1 hunks)chat-android/src/main/java/com/ably/chat/Room.kt(1 hunks)chat-android/src/main/java/com/ably/chat/RoomOptions.kt(6 hunks)chat-android/src/main/java/com/ably/chat/RoomReactions.kt(2 hunks)chat-android/src/main/java/com/ably/chat/RoomStatus.kt(4 hunks)chat-android/src/main/java/com/ably/chat/Rooms.kt(3 hunks)chat-android/src/main/java/com/ably/chat/Subscription.kt(1 hunks)chat-android/src/main/java/com/ably/chat/Typing.kt(1 hunks)chat-android/src/main/java/com/ably/chat/Utils.kt(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- chat-android/src/main/java/com/ably/chat/Utils.kt
🧰 Additional context used
🧠 Learnings (1)
chat-android/src/main/java/com/ably/chat/Rooms.kt (1)
Learnt from: sacOO7
PR: ably/ably-chat-kotlin#84
File: chat-android/src/main/java/com/ably/chat/Rooms.kt:117-126
Timestamp: 2024-12-12T09:18:53.834Z
Learning: In `Rooms.kt` of the `ably-chat-kotlin` project, all operations are executed under `sequentialScope` with parallelism set to `1`, ensuring that concurrency issues are mitigated. Potential race conditions in methods like `release` are not a concern.
🪛 GitHub Actions: Check
chat-android/src/main/java/com/ably/chat/ChatClientOptions.kt
[warning] 6-6: ChatClientOptions is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/OrderBy.kt
[warning] 6-6: OrderBy is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/LogLevel.kt
[warning] 6-6: LogLevel is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Logger.kt
[warning] 8-8: LogHandler is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 9-9: log is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 12-12: LogContext is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Subscription.kt
[warning] 7-7: Subscription is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 11-11: unsubscribe is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Message.kt
[warning] 20-20: Message is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/ChatClient.kt
[warning] 11-11: ChatClient is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 15-15: rooms is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 21-21: connection is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 26-26: clientId is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 31-31: realtime is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 36-36: clientOptions is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Rooms.kt
[warning] 14-14: Rooms is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 19-19: clientOptions is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 40-40: get is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 55-55: release is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Occupancy.kt
[warning] 49-49: Listener is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 24-24: Occupancy is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 30-30: channel is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 37-37: subscribe is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 44-44: get is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 54-54: onEvent is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 63-63: OccupancyEvent is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Room.kt
[warning] 15-15: Room is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 20-20: roomId is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 27-27: messages is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 35-35: presence is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 43-43: reactions is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 51-51: typing is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 59-59: occupancy is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 66-66: options is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 74-74: status is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 79-79: error is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 86-86: onStatusChange is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 91-91: offAllStatusChange is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 103-103: attach is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 108-108: detach is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/PaginatedResult.kt
[warning] 14-14: PaginatedResult is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 19-19: items is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 24-24: next is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 31-31: hasNext is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/RoomReactions.kt
[warning] 54-54: Listener is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 18-18: RoomReactions is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 25-25: channel is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 41-41: send is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 49-49: subscribe is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 59-59: onReaction is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/EventTypes.kt
[warning] 8-8: MessageEventType is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 24-24: ChatMessage is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 54-54: PresenceEventType is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 76-76: TypingEventType is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 85-85: RoomReactionEventType is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Discontinuities.kt
[warning] 36-36: Listener is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 26-26: EmitsDiscontinuities is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 31-31: onDiscontinuity is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 41-41: discontinuityEmitted is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/RoomOptions.kt
[warning] 9-9: RoomOptions is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 39-39: default is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 51-51: PresenceOptions is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 72-72: TypingOptions is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 87-87: RoomReactionsOptions is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 98-98: OccupancyOptions is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/RoomStatus.kt
[warning] 10-10: RoomStatus is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 70-70: RoomStatusChange is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 114-114: Listener is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 91-91: RoomLifecycle is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 96-96: status is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 102-102: error is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 109-109: onChange is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 119-119: roomStatusChanged is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Connection.kt
[warning] 24-24: ConnectionStatus is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 59-59: ConnectionStatusChange is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 106-106: Listener is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 85-85: Connection is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 89-89: status is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 94-94: error is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 101-101: onStatusChange is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 111-111: connectionStatusChanged is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Reaction.kt
[warning] 16-16: Reaction is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Messages.kt
[warning] 86-89: Parameter should start on a newline [ParameterListWrapping]
[warning] 89-89: Missing newline before ')' [ParameterListWrapping]
[warning] 89-89: Missing trailing comma before ')' [TrailingCommaOnDeclarationSite]
chat-android/src/main/java/com/ably/chat/ErrorCodes.kt
[warning] 6-6: ErrorCode is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 123-123: BadRequest is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 125-125: Unauthorized is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 127-127: InternalServerError is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 129-129: NotImplemented is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 131-131: ServiceUnavailable is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 133-133: GatewayTimeout is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 135-135: Timeout is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Typing.kt
[warning] 82-82: Listener is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 46-46: Typing is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 51-51: channel is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 58-58: subscribe is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 64-64: get is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 71-71: start is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 77-77: stop is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 87-87: onEvent is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 94-94: TypingEvent is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Presence.kt
[warning] 75-75: Listener is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 21-21: Presence is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 26-26: channel is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 36-36: get is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 43-43: isUserPresent is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 50-50: enter is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 57-57: update is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 64-64: leave is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 70-70: subscribe is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 80-80: onEvent is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 87-87: PresenceMember is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 117-117: PresenceEvent is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
🔇 Additional comments (57)
chat-android/src/main/java/com/ably/chat/Subscription.kt (1)
7-7:✅ Verification successful
These explicit public modifiers are required with
explicitApimode.While the pipeline warnings flag these explicit
publicmodifiers as redundant (since Kotlin classes and functions are public by default), they are actually necessary when enablingexplicitApimode. This mode requires explicit visibility modifiers on all API declarations, which is exactly what this PR aims to accomplish.The explicit visibility modifiers improve API clarity for library consumers by making public API surface intentional and documented at the declaration site.
Also applies to: 11-11
🏁 Script executed:
#!/bin/bash # Verify that the explicitApi option is being enabled in the build configuration # Look for explicitApi configuration in Gradle files echo "Searching for explicitApi configuration in Gradle files:" rg "explicitApi" --type groovy --type kotlinLength of output: 215
Explicit public modifiers validated with explicitApi enabled
The Gradle configuration confirms that
explicitApiis enabled (e.g., inchat-android/build.gradle.ktsviaexplicitApi()), so the explicitpublicmodifiers in the API declarations (like inSubscription.kt) are indeed required for clarity and API stability.
- Verified explicitApi configuration in
chat-android/build.gradle.kts.- No further changes are needed in
chat-android/src/main/java/com/ably/chat/Subscription.kt.🧰 Tools
🪛 GitHub Actions: Check
[warning] 7-7: Subscription is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Typing.kt (8)
46-46: Visibility modifier aligns with explicitApi configurationThe
publicmodifier on theTypinginterface aligns with enabling theexplicitApimode, which requires explicit visibility modifiers for APIs. While the CI shows this as redundant under standard Kotlin rules, it's necessary when usingexplicitApi.🧰 Tools
🪛 GitHub Actions: Check
[warning] 46-46: Typing is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
51-51: Property visibility explicitly definedThe explicit
publicmodifier on thechannelproperty follows theexplicitApirequirements. This ensures that API consumers clearly understand what's part of the public interface.🧰 Tools
🪛 GitHub Actions: Check
[warning] 51-51: channel is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
58-58: Method visibility properly specifiedThe explicit
publicmodifiers on these methods (subscribe,get,start,stop) clearly indicate they're part of the public API. While marked as redundant by the linter, these modifiers are required byexplicitApimode.Also applies to: 64-64, 71-71, 77-77
🧰 Tools
🪛 GitHub Actions: Check
[warning] 58-58: subscribe is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
82-82: Listener interface properly annotatedThe
Listenerinterface and itsonEventmethod are correctly marked aspublicto fulfill theexplicitApirequirement, ensuring they're explicitly part of the library's public API.Also applies to: 87-87
🧰 Tools
🪛 GitHub Actions: Check
[warning] 82-82: Listener is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
94-94: Data class visibility matches explicitApi requirementsThe
TypingEventdata class is appropriately marked with thepublicmodifier to comply withexplicitApimode, making it clear this class is meant for external use.🧰 Tools
🪛 GitHub Actions: Check
[warning] 94-94: TypingEvent is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
28-38: Constants correctly remain internalThe constants related to presence retry logic (
PRESENCE_GET_RETRY_INTERVAL_MS,PRESENCE_GET_RETRY_MAX_INTERVAL_MS,PRESENCE_GET_MAX_RETRIES) are correctly maintained asinternal, keeping implementation details hidden from API consumers.
96-96: Implementation class properly encapsulatedThe
DefaultTypingclass is correctly kept asinternal, ensuring that only the interface is exposed to API consumers while the implementation remains hidden. This follows good API design principles.
46-94:❓ Verification inconclusive
Consider addressing pipeline warnings
While the
publicmodifiers are necessary forexplicitApimode, the CI pipeline is flagging them as redundant. Consider one of these approaches:
- Add a suppression annotation for these specific warnings
- Configure the linter to account for
explicitApimode- Document in the PR description why these warnings are expected and can be ignored
This would help maintain a clean CI pipeline.
🌐 Web query:
How to suppress redundant visibility modifier warnings in Kotlin with explicitApi?Error during web search
Address CI Warning for Redundant Visibility Modifiers
While the code complies with
explicitApirequirements by marking methods and interfaces aspublic, the CI pipeline issues warnings for their redundancy. To remedy this, please consider one of the following approaches:
- Add a Suppression Annotation: You can suppress these specific warnings by adding a file-level annotation (e.g.,
@file:Suppress("RedundantVisibilityModifier")) at the top of the affected file.- Adjust Linter Configuration: Update the linter rules to account for
explicitApimode so that these warnings are not flagged.- Document the Exception: If you prefer to leave the warnings in place, please document in the PR description that these warnings are expected due to
explicitApiusage.This will help maintain a clean CI output while justifying the explicit modifiers.
🧰 Tools
🪛 GitHub Actions: Check
[warning] 82-82: Listener is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 46-46: Typing is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 51-51: channel is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 58-58: subscribe is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 64-64: get is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 71-71: start is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 77-77: stop is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 87-87: onEvent is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 94-94: TypingEvent is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/build.gradle.kts (1)
46-48: Excellent addition of the explicit API mode configuration.Adding
explicitApi()is a good practice for library developers as it enforces explicit visibility modifiers for public API elements. This helps prevent unintentional exposure of internal APIs and creates a clearer API contract for users of your library.chat-android/src/main/java/com/ably/chat/ChatClientOptions.kt (1)
6-6: Thepublicmodifier is technically redundant but required for explicitApi mode.The pipeline reports that this public modifier is redundant since public is the default visibility for classes in Kotlin. However, this is intentional and required when using
explicitApi()mode, which enforces explicit visibility declarations for all API elements.🧰 Tools
🪛 GitHub Actions: Check
[warning] 6-6: ChatClientOptions is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/OrderBy.kt (1)
6-6: Thepublicmodifier is technically redundant but required for explicitApi mode.Similar to the other files, the pipeline reports that this public modifier is redundant. However, with
explicitApi()enabled, this explicit declaration is necessary and appropriate.🧰 Tools
🪛 GitHub Actions: Check
[warning] 6-6: OrderBy is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Headers.kt (1)
19-19: Clear visibility declaration for a type alias used in the public API.Adding the explicit
publicmodifier to this type alias ensures it's properly exposed as part of your library's public API. This is consistent with the explicit API mode approach.chat-android/src/main/java/com/ably/chat/Metadata.kt (1)
18-18: Visibility modifier correctly appliedThe public visibility modifier has been correctly added to the
Metadatatype alias, making it explicitly accessible outside the package. This is consistent with the PR objective of enablingexplicitApimode for the Kotlin project.chat-android/src/main/java/com/ably/chat/OperationMetadata.kt (1)
11-11: Visibility modifier correctly appliedThe public visibility modifier has been correctly added to the
OperationMetadatatype alias, making it explicitly accessible outside the package. This aligns with the PR objective of enablingexplicitApimode for the Kotlin project.chat-android/src/main/java/com/ably/chat/LogLevel.kt (1)
6-6: Redundant visibility modifier detected by pipelineWhile adding the
publicvisibility modifier to the enum class is consistent with the PR objective of enablingexplicitApimode, the pipeline has flagged it as redundant sincepublicis the default visibility for classes in Kotlin.Since the purpose of this PR is to make all API elements explicit, keeping this modifier for consistency is acceptable, but be aware of the warning.
The pipeline shows:
[warning] 6-6: LogLevel is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]Would you like me to investigate if there's a way to suppress this warning, given that it's intentional as part of enabling
explicitApi?🧰 Tools
🪛 GitHub Actions: Check
[warning] 6-6: LogLevel is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Logger.kt (2)
8-9: Redundant visibility modifiers detected by pipelineWhile adding the
publicvisibility modifiers to theLogHandlerinterface and itslogmethod is consistent with the PR objective of enablingexplicitApimode, the pipeline has flagged these as redundant sincepublicis the default visibility for classes and functions in Kotlin.Since the purpose of this PR is to make all API elements explicit, keeping these modifiers for consistency is acceptable, but be aware of the warnings.
The pipeline shows:
[warning] 8-8: LogHandler is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule] [warning] 9-9: log is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]These warnings are expected when enabling
explicitApimode and making previously implicit public APIs explicit. The Kotlin compiler will now enforce explicit visibility modifiers for all API elements, which is the goal of this PR.🧰 Tools
🪛 GitHub Actions: Check
[warning] 8-8: LogHandler is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 9-9: log is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
12-16: Redundant visibility modifier detected by pipelineWhile adding the
publicvisibility modifier to theLogContextdata class is consistent with the PR objective, the pipeline has flagged it as redundant sincepublicis the default visibility for classes in Kotlin.Since the purpose of this PR is to make all API elements explicit, keeping this modifier for consistency is acceptable, but be aware of the warning.
The pipeline shows:
[warning] 12-12: LogContext is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]This warning is expected as part of the
explicitApimode enablement process.🧰 Tools
🪛 GitHub Actions: Check
[warning] 12-12: LogContext is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/PaginatedResult.kt (1)
14-14: LGTM: Making interface members explicitly public is aligned with theexplicitApisetting.These explicit visibility modifiers are part of turning on the
explicitApiKotlin feature, which helps prevent unintentional exposure of APIs by requiring all public APIs to be explicitly marked. The pipeline warnings about redundant modifiers are expected, since public is the default visibility in Kotlin, but they're necessary for this feature.Also applies to: 19-19, 24-24, 31-31
🧰 Tools
🪛 GitHub Actions: Check
[warning] 14-14: PaginatedResult is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/RoomStatus.kt (2)
10-10: LGTM: Making enum class and interfaces explicitly public is aligned with theexplicitApisetting.These explicit visibility modifiers are part of turning on the
explicitApiKotlin feature, which helps prevent unintentional exposure of APIs by requiring all public APIs to be explicitly marked. The pipeline warnings about redundant modifiers are expected, since public is the default visibility in Kotlin, but they're necessary for this feature.Also applies to: 70-70, 91-91, 96-96, 102-102, 109-109, 114-114, 119-119
🧰 Tools
🪛 GitHub Actions: Check
[warning] 10-10: RoomStatus is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
197-197:✅ Verification successful
Verify that removing the
overridemodifier is intentional.The
overridemodifier has been removed from theoffAll()function. This suggests that this function is no longer overriding a method from a parent class or interface. Make sure this change is intentional and that it doesn't break any functionality.
🏁 Script executed:
#!/bin/bash # Checking if InternalRoomLifecycle defines offAll() method ast-grep -p $'interface InternalRoomLifecycle { $$$ fun offAll() $$$ }' # Checking parent interfaces/classes for override rg -A 5 -B 5 "fun offAll\(\)" --glob "*.kt"Length of output: 979
OffAll() Override Removal Confirmed
The verification confirms that no parent class or interface (including an expected InternalRoomLifecycle interface) declares an offAll() method. In the file
chat-android/src/main/java/com/ably/chat/RoomStatus.kt, the offAll() function is defined without the override modifier, while other functions likeonChangeOnce()still correctly use it. This indicates that the removal of the override modifier from offAll() is intentional and does not break any inherited contract.chat-android/src/main/java/com/ably/chat/Message.kt (1)
10-10: LGTM: Making type aliases and data class explicitly public is aligned with theexplicitApisetting.These explicit visibility modifiers are part of turning on the
explicitApiKotlin feature, which helps prevent unintentional exposure of APIs by requiring all public APIs to be explicitly marked. The pipeline warnings about redundant modifiers are expected, since public is the default visibility in Kotlin, but they're necessary for this feature.Also applies to: 15-15, 20-20
chat-android/src/main/java/com/ably/chat/Reaction.kt (1)
6-6: LGTM: Making type aliases and data class explicitly public is aligned with theexplicitApisetting.These explicit visibility modifiers are part of turning on the
explicitApiKotlin feature, which helps prevent unintentional exposure of APIs by requiring all public APIs to be explicitly marked. The pipeline warnings about redundant modifiers are expected, since public is the default visibility in Kotlin, but they're necessary for this feature.Also applies to: 11-11, 16-16
chat-android/src/main/java/com/ably/chat/ChatClient.kt (3)
11-11: Visibility modification: Interface marked as publicThe
ChatClientinterface has been explicitly marked as public, which aligns with theexplicitApiconfiguration enabled in this PR. This helps document the intended API surface of the library.Note that the static analysis reports this as redundant since
publicis the default visibility for interfaces in Kotlin, but it's required when usingexplicitApimode.🧰 Tools
🪛 GitHub Actions: Check
[warning] 11-11: ChatClient is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
15-15: Visibility modifications: Properties marked as publicAll properties within the
ChatClientinterface have been explicitly marked as public. While these modifiers are technically redundant (as reported by the static analysis), they're required when usingexplicitApimode to clearly define the API surface.Also applies to: 21-21, 26-26, 31-31, 36-36
🧰 Tools
🪛 GitHub Actions: Check
[warning] 15-15: rooms is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
39-39: Visibility modification: Function marked as publicThe factory function
ChatClienthas been explicitly marked as public, which aligns with theexplicitApiconfiguration. This makes it clear that this function is part of the public API.chat-android/src/main/java/com/ably/chat/Rooms.kt (4)
14-14: Visibility modification: Interface marked as publicThe
Roomsinterface has been explicitly marked as public, which aligns with theexplicitApiconfiguration enabled in this PR. This helps document the intended API surface of the library.Note that the static analysis reports this as redundant since
publicis the default visibility for interfaces in Kotlin, but it's required when usingexplicitApimode.🧰 Tools
🪛 GitHub Actions: Check
[warning] 14-14: Rooms is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
19-19: Visibility modification: Property marked as publicThe
clientOptionsproperty has been explicitly marked as public. While this modifier is technically redundant (as reported by the static analysis), it's required when usingexplicitApimode to clearly define the API surface.🧰 Tools
🪛 GitHub Actions: Check
[warning] 19-19: clientOptions is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
40-40: Visibility modification: Methods marked as publicThe
getandreleasemethods have been explicitly marked as public. This makes it clear that these methods are part of the public API surface of the library, which is important for API documentation.While these modifiers are reported as redundant by the static analysis, they're required when using
explicitApimode.Also applies to: 55-55
🧰 Tools
🪛 GitHub Actions: Check
[warning] 40-40: get is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
115-126: Robust concurrency control in room operationsThis implementation shows good practices for concurrency control. All operations are executed under
sequentialScopewith parallelism limited to 1, ensuring that only one coroutine runs at a time. This effectively prevents race conditions in methods likerelease.No changes are needed, just highlighting a well-designed aspect of the code.
chat-android/src/main/java/com/ably/chat/Discontinuities.kt (2)
31-31: API visibility change: Methods and nested interface changed from internal to publicThe
onDiscontinuitymethod,Listenerinterface, anddiscontinuityEmittedmethod have all been changed frominternaltopublic. This expands the library's API surface by exposing these elements for external use.Since these are part of the
EmitsDiscontinuitiesinterface (now public), it makes sense that they're also public. Ensure that these methods and the nested interface are properly documented and have stable signatures since they're now part of the public API.Also applies to: 36-36, 41-41
🧰 Tools
🪛 GitHub Actions: Check
[warning] 31-31: onDiscontinuity is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
26-26:❓ Verification inconclusive
API visibility change: Interface changed from internal to public
The
EmitsDiscontinuitiesinterface has been changed frominternaltopublic. This is more than just an explicit visibility declaration - it's actually expanding the library's API surface by exposing a previously internal interface.Verify that this interface is intended to be part of the public API and that it's properly documented for external use.
🏁 Script executed:
#!/bin/bash # Look for implementations of EmitsDiscontinuities that might now be exposed echo "Searching for implementations of EmitsDiscontinuities..." rg -A 2 "class .+ : .+EmitsDiscontinuities" echo "Checking if this interface is used in other public interfaces..." rg -A 2 "interface .+ : .+EmitsDiscontinuities"Length of output: 716
API Visibility Change: Verify Public Exposure of EmitsDiscontinuities
It looks like the interface’s visibility was changed from internal to public, which expands the API surface. The repository search shows that the implementation (i.e. the internal class
DiscontinuityEmitterin the same file) uses the interface, with no additional occurrences in other public interfaces. Please confirm that exposingEmitsDiscontinuitiesas part of the public API is intentional and that all necessary documentation and support for external use is in place.
- File:
chat-android/src/main/java/com/ably/chat/Discontinuities.ktat line 26- Implementation found:
internal class DiscontinuityEmitter(...) : EventEmitter<String, EmitsDiscontinuities.Listener>()- Action needed: Verify whether this API change aligns with your intended design, and ensure comprehensive API documentation exists.
🧰 Tools
🪛 GitHub Actions: Check
[warning] 26-26: EmitsDiscontinuities is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Room.kt (4)
15-15: Visibility modification: Interface marked as publicThe
Roominterface has been explicitly marked as public, which aligns with theexplicitApiconfiguration enabled in this PR. This helps document the intended API surface of the library.Note that the static analysis reports this as redundant since
publicis the default visibility for interfaces in Kotlin, but it's required when usingexplicitApimode.🧰 Tools
🪛 GitHub Actions: Check
[warning] 15-15: Room is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
20-20: Visibility modification: Properties marked as publicAll properties within the
Roominterface have been explicitly marked as public. While these modifiers are technically redundant (as reported by the static analysis), they're required when usingexplicitApimode to clearly define the API surface.Also applies to: 27-27, 35-35, 43-43, 51-51, 59-59, 66-66, 74-74, 79-79
🧰 Tools
🪛 GitHub Actions: Check
[warning] 20-20: roomId is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
86-86: Visibility modification: Methods marked as publicAll methods in the
Roominterface (onStatusChange,offAllStatusChange,attach, anddetach) have been explicitly marked as public. This makes it clear that these methods are part of the public API surface of the library.While these modifiers are reported as redundant by the static analysis, they're required when using
explicitApimode.Also applies to: 91-91, 103-103, 108-108
🧰 Tools
🪛 GitHub Actions: Check
[warning] 86-86: onStatusChange is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
96-96: Documentation update: Reference to RoomStatus instead of RoomLifecycleThe comment has been updated to reference
RoomStatusinstead ofRoomLifecyclefor the states a room can enter upon failure to attach. This is a good documentation fix that ensures consistency in terminology.chat-android/src/main/java/com/ably/chat/RoomOptions.kt (2)
88-90: Added explicit return types for equals and hashCode in RoomReactionsOptionsGood job adding explicit return types to the
equalsandhashCodemethods. This is required byexplicitApi()and improves code clarity.
99-101: Added explicit return types for equals and hashCode in OccupancyOptionsGood job adding explicit return types to the
equalsandhashCodemethods. This is required byexplicitApi()and improves code clarity.chat-android/src/main/java/com/ably/chat/RoomReactions.kt (1)
41-41: Verified interface method signature consistencyThe
sendmethod has been properly annotated with thepublicmodifier and maintains its original signature. This ensures API compatibility while making the visibility explicit.🧰 Tools
🪛 GitHub Actions: Check
[warning] 41-41: send is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Messages.kt (1)
123-123: Redundant public modifiers on data classesThe
QueryOptionsandMessageEventdata classes are explicitly marked as public, which is redundant as public is the default visibility. This is expected when usingexplicitApi().Also applies to: 154-154
chat-android/src/main/java/com/ably/chat/Connection.kt (1)
19-19: Maintain internal constant visibilityGood job maintaining the
internalvisibility modifier for theTRANSIENT_TIMEOUTconstant. This correctly prevents it from being exposed as part of the public API.chat-android/src/main/java/com/ably/chat/EventTypes.kt (5)
8-8: The explicit public modifiers are intentional for theexplicitApimode.These explicit
publicmodifiers are flagged as redundant by the linter because Kotlin classes are public by default. However, they're being added intentionally to support theexplicitApimode that's being enabled in this PR, which requires explicit visibility modifiers on all public API elements.🧰 Tools
🪛 GitHub Actions: Check
[warning] 8-8: MessageEventType is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
22-24: Correct public API visibility for PubSubEventName.The explicit
publicmodifiers here are part of theexplicitApimode implementation. This change appropriately exposes thePubSubEventNameobject and its constant to library users, making the API boundaries clearer.🧰 Tools
🪛 GitHub Actions: Check
[warning] 24-24: ChatMessage is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
54-54: Explicit visibility for PresenceEventType follows the pattern.This is consistent with the other enum classes in the file, making the API contract explicit as required by
explicitApimode.🧰 Tools
🪛 GitHub Actions: Check
[warning] 54-54: PresenceEventType is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
76-76: Explicit visibility for TypingEventType follows the established pattern.Making the visibility explicit here is consistent with the approach for other enums and supports the
explicitApimode.🧰 Tools
🪛 GitHub Actions: Check
[warning] 76-76: TypingEventType is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
85-85: Consistent public API exposure for RoomReactionEventType.The explicit
publicmodifier is redundant in standard Kotlin code but necessary forexplicitApimode. This follows the same pattern as other enums in this file.🧰 Tools
🪛 GitHub Actions: Check
[warning] 85-85: RoomReactionEventType is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/ErrorCodes.kt (3)
6-6: Explicit visibility for ErrorCode aligns with explicitApi mode.Adding the explicit
publicmodifier to this enum class and its property is part of implementing theexplicitApirequirement, making it clear that this is part of the library's public API.🧰 Tools
🪛 GitHub Actions: Check
[warning] 6-6: ErrorCode is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
121-121: HttpStatusCode exposed as public API.The explicit
publicmodifier for the HttpStatusCode object makes it clear that this is an intentionally exposed part of the library's API, which is the goal of enablingexplicitApimode.
123-135: Status code constants are appropriately exposed as public API.The explicit
publicmodifiers and type annotations for these constants are consistent with theexplicitApiapproach. The type annotations make the API contract more explicit, which is beneficial for library consumers.🧰 Tools
🪛 GitHub Actions: Check
[warning] 123-123: BadRequest is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 125-125: Unauthorized is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 127-127: InternalServerError is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 129-129: NotImplemented is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 131-131: ServiceUnavailable is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 133-133: GatewayTimeout is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 135-135: Timeout is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Presence.kt (6)
13-13: Public type alias for PresenceData.Making this type alias explicitly public clarifies that it's intended to be used by external code, which is appropriate given its role in the public API.
21-26: Presence interface and its properties are now explicitly part of the public API.The explicit
publicmodifiers for the interface and its property make it clear that these are intended to be used by library consumers. This is consistent with theexplicitApiapproach.🧰 Tools
🪛 GitHub Actions: Check
[warning] 21-21: Presence is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 26-26: channel is explicitly marked as public. Properties are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
36-70: All Presence methods are now explicitly marked as public.Adding explicit
publicmodifiers to these methods makes the API contract clearer, which is beneficial for users of the library. The consistency across all methods is particularly helpful.🧰 Tools
🪛 GitHub Actions: Check
[warning] 36-36: get is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 43-43: isUserPresent is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 50-50: enter is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 57-57: update is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 64-64: leave is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 70-70: subscribe is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
75-81: Listener interface and its method are appropriately exposed.Making the Listener interface and its onEvent method explicitly public ensures that users can implement this interface to respond to presence events, which is an important part of the library's functionality.
🧰 Tools
🪛 GitHub Actions: Check
[warning] 75-75: Listener is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
[warning] 80-80: onEvent is explicitly marked as public. Functions are public by default so this modifier is redundant. [RedundantVisibilityModifierRule]
87-112: PresenceMember data class is now explicitly public.This data class represents presence information that needs to be accessible to users of the library, so making it explicitly public is appropriate.
🧰 Tools
🪛 GitHub Actions: Check
[warning] 87-87: PresenceMember is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
117-137: PresenceEvent data class is properly exposed as public API.This data class needs to be accessible to library users since it's passed to event listeners, making its explicit public visibility appropriate and consistent with the
explicitApiapproach.🧰 Tools
🪛 GitHub Actions: Check
[warning] 117-117: PresenceEvent is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
chat-android/src/main/java/com/ably/chat/Occupancy.kt (2)
24-24: Explicit API mode now requires public visibility modifiersAll the additions of explicit
publicmodifiers align with turning on theexplicitApimode in Kotlin. While these modifiers would be redundant in regular Kotlin code (as they're the default), theexplicitApimode requires explicit visibility modifiers for APIs to improve API clarity in libraries.The pipeline is still flagging these as redundant visibility modifiers, but this is expected with the new configuration - the linting rules may need to be updated to accommodate the explicit API requirement.
Also applies to: 30-30, 37-37, 44-44, 49-49, 54-54, 63-63
🧰 Tools
🪛 GitHub Actions: Check
[warning] 24-24: Occupancy is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
75-75: Appropriate visibility change for internal constantChanging the
META_OCCUPANCY_EVENT_NAMEconstant toprivateis the correct approach. Since this constant is only used within this file (for internal channel subscription on line 116), it should not be part of the public API.
| @@ -21,37 +21,37 @@ import kotlinx.coroutines.launch | |||
| * | |||
| * Get an instance via [Room.occupancy]. | |||
| */ | |||
| interface Occupancy : EmitsDiscontinuities { | |||
| public interface Occupancy : EmitsDiscontinuities { | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Configure the linter to recognize explicitApi mode
The pipeline failures show that the linter is flagging the public modifiers as redundant, but they're actually required with explicitApi. Consider updating the linter rules to be compatible with explicit API mode.
Also applies to: 30-30, 37-37, 44-44, 49-49, 54-54, 63-63
🏁 Script executed:
#!/bin/bash
# Check if the detekt configuration can be configured for explicitApi
# Find detekt configuration files
echo "Looking for detekt configuration files:"
fd -e yml -e yaml detekt
# Check if there's a .editorconfig file that might configure style rules
echo -e "\nLooking for .editorconfig:"
fd .editorconfig
# Check build.gradle or build.gradle.kts files for explicitApi configuration
echo -e "\nChecking Gradle files for explicitApi configuration:"
fd build.gradle build.gradle.kts | xargs grep -l "explicitApi"Length of output: 521
Linter Configuration for Explicit API Mode
It appears that our Detekt configuration files (e.g., detekt-config.yaml and detekt.yml) are present, but our initial check for explicit API mode settings in the Gradle build scripts was inconclusive due to command errors. The pipeline failures indicate that Detekt is flagging required public modifiers as redundant under explicit API mode. To address this, please ensure one of the following:
- Update the Detekt rules (or its configuration) to recognize that explicit API mode requires explicit
publicmodifiers. - Alternatively, if explicit API mode is intended to be enabled, verify that the Kotlin
explicitApi()setting is correctly activated in your Gradle build files.
This change should be applied at all affected locations in
chat-android/src/main/java/com/ably/chat/Occupancy.kt (lines 24, 30, 37, 44, 49, 54, and 63).
To further verify the Gradle side, please run the following script to search for any explicit API configuration in your build files:
Once confirmed, adjust either your Gradle or Detekt configuration to resolve the pipeline failures.
🏁 Script executed:
#!/bin/bash
echo "Searching for 'explicitApi' configuration in Gradle build files:"
find . -type f \( -name "build.gradle" -o -name "build.gradle.kts" \) -exec grep -Hn "explicitApi" {} +Length of output: 423
Update Detekt Configuration to Support Explicit API Mode
We've verified that explicit API mode is enabled in your Gradle configuration (see chat-android/build.gradle.kts:47) and that Detekt configuration files (detekt-config.yaml and detekt.yml) are present. However, the Detekt linter still flags the explicitly required public modifiers as redundant, causing pipeline failures.
Action Items:
- Review and update Detekt rules: Modify your Detekt configuration to recognize explicit API mode so that the
publicmodifiers are not reported as redundant. - Apply changes to all affected instances: This affects the
Occupancy.ktfile on lines 24, 30, 37, 44, 49, 54, and 63.
Please verify that the updated Detekt configuration properly acknowledges explicit API requirements.
🧰 Tools
🪛 GitHub Actions: Check
[warning] 24-24: Occupancy is explicitly marked as public. Public is the default visibility for classes. The public modifier is redundant. [RedundantVisibilityModifierRule]
|
Seems we need to Suppress |
ttypic
force-pushed
the
turn-on-explicit-api
branch
from
870168c to
aef0111
Compare
The `explicitApi` option is recommended for library developers and helps prevent the exposure of internal APIs.
ttypic
force-pushed
the
turn-on-explicit-api
branch
from
aef0111 to
5f4694a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
chat-android/src/test/java/com/ably/chat/TestUtils.kt (1)
16-22: 🛠️ Refactor suggestionAdd explicit
publicvisibility modifier for better API claritySince the PR's objective is to turn on
explicitApifor Kotlin, all public functions should have explicit visibility modifiers. Most utility functions in this file lack thepublicmodifier. While they are public by default, adding explicit modifiers would align with the PR objectives.For example:
- fun buildAsyncHttpPaginatedResponse(items: List<JsonElement>): AsyncHttpPaginatedResponse { + public fun buildAsyncHttpPaginatedResponse(items: List<JsonElement>): AsyncHttpPaginatedResponse {Apply this change pattern to all public utility functions in the file.
Also applies to: 24-33, 35-46, 48-59, 66-72, 74-83, 85-89, 91-96, 112-118
🧹 Nitpick comments (3)
chat-android/src/test/java/com/ably/chat/TestUtils.kt (2)
103-103: Non-null assertion operator is redundant here.The
!!operator is redundant because the while loop condition on line 101 already checkscurrentClass.superclass != null. This means the code will only reach line 103 ifsuperclassis not null. While adding the non-null assertion aligns with making the API more explicit, it's unnecessary in this case and could be removed to keep the code cleaner.- currentClass = currentClass.superclass!! + currentClass = currentClass.superclass
98-110: Consider making thefindFieldmethod more robustThe current implementation of
findFielduses exceptions for control flow, which is not ideal for performance and readability. Consider refactoring to use a more functional approach with a loop that doesn't rely on exception handling.private fun Class<*>.findField(name: String): Field { - var result = kotlin.runCatching { getDeclaredField(name) } - var currentClass = this - while (result.isFailure && currentClass.superclass != null) // stop when we got field or reached top of class hierarchy - { - currentClass = currentClass.superclass!! - result = kotlin.runCatching { currentClass.getDeclaredField(name) } - } - if (result.isFailure) { - throw result.exceptionOrNull() as Exception - } - return result.getOrNull() as Field + var currentClass: Class<*>? = this + while (currentClass != null) { + try { + return currentClass.getDeclaredField(name) + } catch (e: NoSuchFieldException) { + currentClass = currentClass.superclass + } + } + throw NoSuchFieldException("Field $name not found in class hierarchy of ${this.name}") }chat-android/src/main/java/com/ably/chat/Room.kt (1)
20-20: Review public property exposures
All these properties (roomId,messages,presence,reactions,typing,occupancy,options,status, anderror) are now explicitly declared aspublic. Ensure that broad visibility is truly desired. If not, useinternalor reduce visibility. Also note that you may need to suppress theRedundantVisibilityModifierlint rule if it triggers conflicts underexplicitApi.Also applies to: 27-27, 35-35, 43-43, 51-51, 59-59, 66-66, 74-74, 79-79
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
chat-android/build.gradle.kts(1 hunks)chat-android/src/main/java/com/ably/chat/ChatClient.kt(1 hunks)chat-android/src/main/java/com/ably/chat/ChatClientOptions.kt(1 hunks)chat-android/src/main/java/com/ably/chat/Connection.kt(3 hunks)chat-android/src/main/java/com/ably/chat/Discontinuities.kt(1 hunks)chat-android/src/main/java/com/ably/chat/ErrorCodes.kt(2 hunks)chat-android/src/main/java/com/ably/chat/EventTypes.kt(5 hunks)chat-android/src/main/java/com/ably/chat/Headers.kt(1 hunks)chat-android/src/main/java/com/ably/chat/LogLevel.kt(1 hunks)chat-android/src/main/java/com/ably/chat/Logger.kt(1 hunks)chat-android/src/main/java/com/ably/chat/Message.kt(1 hunks)chat-android/src/main/java/com/ably/chat/Messages.kt(7 hunks)chat-android/src/main/java/com/ably/chat/Metadata.kt(1 hunks)chat-android/src/main/java/com/ably/chat/Occupancy.kt(3 hunks)chat-android/src/main/java/com/ably/chat/OperationMetadata.kt(1 hunks)chat-android/src/main/java/com/ably/chat/OrderBy.kt(1 hunks)chat-android/src/main/java/com/ably/chat/PaginatedResult.kt(1 hunks)chat-android/src/main/java/com/ably/chat/Presence.kt(3 hunks)chat-android/src/main/java/com/ably/chat/Reaction.kt(1 hunks)chat-android/src/main/java/com/ably/chat/Room.kt(2 hunks)chat-android/src/main/java/com/ably/chat/RoomOptions.kt(6 hunks)chat-android/src/main/java/com/ably/chat/RoomReactions.kt(2 hunks)chat-android/src/main/java/com/ably/chat/RoomStatus.kt(7 hunks)chat-android/src/main/java/com/ably/chat/Rooms.kt(3 hunks)chat-android/src/main/java/com/ably/chat/Subscription.kt(1 hunks)chat-android/src/main/java/com/ably/chat/Typing.kt(1 hunks)chat-android/src/main/java/com/ably/chat/Utils.kt(1 hunks)chat-android/src/test/java/com/ably/chat/TestUtils.kt(1 hunks)chat-android/src/test/java/com/ably/chat/room/RoomEnsureAttachedTest.kt(3 hunks)detekt.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (27)
- chat-android/src/main/java/com/ably/chat/OrderBy.kt
- chat-android/src/main/java/com/ably/chat/ChatClientOptions.kt
- chat-android/src/main/java/com/ably/chat/LogLevel.kt
- detekt.yml
- chat-android/build.gradle.kts
- chat-android/src/main/java/com/ably/chat/OperationMetadata.kt
- chat-android/src/main/java/com/ably/chat/Utils.kt
- chat-android/src/main/java/com/ably/chat/Subscription.kt
- chat-android/src/main/java/com/ably/chat/Metadata.kt
- chat-android/src/main/java/com/ably/chat/PaginatedResult.kt
- chat-android/src/main/java/com/ably/chat/Message.kt
- chat-android/src/main/java/com/ably/chat/Logger.kt
- chat-android/src/main/java/com/ably/chat/ChatClient.kt
- chat-android/src/main/java/com/ably/chat/Reaction.kt
- chat-android/src/main/java/com/ably/chat/Occupancy.kt
- chat-android/src/main/java/com/ably/chat/Discontinuities.kt
- chat-android/src/main/java/com/ably/chat/Rooms.kt
- chat-android/src/main/java/com/ably/chat/Connection.kt
- chat-android/src/main/java/com/ably/chat/Headers.kt
- chat-android/src/main/java/com/ably/chat/Messages.kt
- chat-android/src/main/java/com/ably/chat/RoomOptions.kt
- chat-android/src/main/java/com/ably/chat/RoomReactions.kt
- chat-android/src/main/java/com/ably/chat/RoomStatus.kt
- chat-android/src/main/java/com/ably/chat/ErrorCodes.kt
- chat-android/src/main/java/com/ably/chat/Presence.kt
- chat-android/src/main/java/com/ably/chat/EventTypes.kt
- chat-android/src/main/java/com/ably/chat/Typing.kt
🧰 Additional context used
🧠 Learnings (1)
chat-android/src/test/java/com/ably/chat/room/RoomEnsureAttachedTest.kt (2)
Learnt from: sacOO7
PR: ably/ably-chat-kotlin#66
File: chat-android/src/main/java/com/ably/chat/Room.kt:0-0
Timestamp: 2024-11-28T11:09:34.042Z
Learning: In the `DefaultRoom` class, the `ensureAttached()` method checks `statusLifecycle.status` again inside `roomScope.launch` to handle possible state changes, ensuring robustness.
Learnt from: sacOO7
PR: ably/ably-chat-kotlin#66
File: chat-android/src/test/java/com/ably/chat/RoomReactionsTest.kt:44-50
Timestamp: 2024-11-28T11:08:42.524Z
Learning: The test cases for verifying behavior when the room is not in the ATTACHED state are covered in `chat-android/src/test/java/com/ably/chat/room/RoomEnsureAttachedTest.kt`.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: coverage
- GitHub Check: check
🔇 Additional comments (11)
chat-android/src/test/java/com/ably/chat/room/RoomEnsureAttachedTest.kt (5)
8-8: Appropriate package structure update reflecting the API reorganization.The import has been updated from
RoomLifecycletoRoomas part of the explicit API reorganization, which appears to be centralizing listener interfaces directly within their parent classes.
90-90: Interface reference updated correctly for explicit API mode.The reference to
RoomLifecycle.Listenerhas been updated toRoom.Listener, correctly reflecting the interface reorganization performed as part of enabling explicit API mode in Kotlin.
92-92: Consistent parameter type update.The parameter extraction has been properly updated to use the new
Room.Listenertype, maintaining consistency with the mocked function parameter type.
104-104: Verification call updated appropriately.The verification of
onChangeOncecall has been updated to check for the newRoom.Listenertype parameter, correctly reflecting the API changes.
30-173: Test class functionality preserved through API reorganization.The test class still properly verifies the behavior of
ensureAttached()in various room states, focusing on the core functionality described in the specs (CHA-RL9, CHA-PR3d, etc.). The changes reflect a structural reorganization where theListenerinterface has been moved fromRoomLifecycleto be nested within theRoomclass, which improves API clarity by keeping related interfaces closer to their primary class.This reorganization maintains all test coverage while adopting a more coherent API design that will be clearer for library consumers.
chat-android/src/main/java/com/ably/chat/Room.kt (6)
15-15: Confirm intention for public interface
DeclaringRoomaspublicunderexplicitApiis understandable. However, if this interface is meant for internal library usage only, consider marking it asinternalto avoid expanding the public API surface unnecessarily.
86-86: Public listener registration method
ExposingonStatusChangepublicly can benefit library consumers but be certain this is intentional, as it expands the API surface.
91-91: Check revised documentation reference
Your doc comment now references[RoomStatus.Suspended]and[RoomStatus.Failed]. Verify there are no remaining references to the oldRoomLifecyclestates elsewhere.
98-98: Publicattach()anddetach()
Explicitly marking these suspend functions aspublicmakes sense for library consumers.Also applies to: 103-103
105-114: Introduction of a public fun interface
DeclaringListeneras apublic fun interfaceis modern and concise. Just ensure your consumers target Kotlin versions supporting fun interfaces (≥1.4). Otherwise, consider a conventional interface if backward compatibility is needed.
224-224: Override matches new listener signature
This override correctly referencesRoom.Listener. Confirm that all references to the oldRoomLifecycle.Listenerhave been removed or updated.
The
explicitApioption is recommended for library developers and helps prevent the exposure of internal APIs.Summary by CodeRabbit
Chores
Refactor