Fix android annotation for jetpack #3195
Conversation
WalkthroughReplaces the "jetpack" language designation with "android" across language configuration and documentation. Updates include renaming keys in languageData, adjusting languageInfo entry (alias and syntaxHighlighterKey), removing entries from languageKeys, and swapping documentation language guards and code-fence labels to "android". Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
sacOO7
force-pushed
the
fix/android-annotation-for-jetpack
branch
3 times, most recently
from
46f3dbc to
2e520e2
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pages/docs/chat/rooms/reactions.mdx (1)
60-73:⚠️ Potential issue | 🟡 MinorMissing
asFlowimport in the reactions subscribe example.Line 68 calls
room.reactions.asFlow(), but the imports on lines 61–63 don't include theasFlowextension. Inmessages.mdx(line 68),com.ably.chat.asFlowis explicitly imported. Add it here for consistency and correctness.Proposed fix
import androidx.compose.runtime.* import com.ably.chat.Room import com.ably.chat.RoomReactionEvent +import com.ably.chat.asFlowsrc/pages/docs/chat/rooms/presence.mdx (1)
213-242:⚠️ Potential issue | 🟡 MinorEmpty
DisposableEffect.onDisposeblock with a TODO-like comment is misleading in documentation.Lines 234–239 show a
DisposableEffectwith an emptyonDisposeblock and a comment suggesting the reader "consider using a coroutine scope for leave()." Documentation examples should either demonstrate the complete pattern or omit the incomplete pattern entirely. Leaving an empty cleanup block may confuse developers trying to implement presence lifecycle management.Consider either implementing the cleanup (e.g., using
rememberCoroutineScopeto callroom.presence.leave()inonDispose) or removing theDisposableEffectblock and adding a prose note about cleanup considerations.Option A: Implement the cleanup
+@Composable +fun EnterPresenceComponent(room: Room) { + val coroutineScope = rememberCoroutineScope() + LaunchedEffect(room) { - // Ensure room is attached before entering presence - if (room.status != RoomStatus.Attached) { - room.attach() - } - room.presence.enter( jsonObject { put("status", "Online") } ) } - // Leave presence when component is disposed DisposableEffect(room) { onDispose { - // Note: Consider using a coroutine scope for leave() - // as DisposableEffect doesn't support suspend functions directly + coroutineScope.launch { + room.presence.leave() + } } } }Option B: Remove incomplete block
`@Composable` fun EnterPresenceComponent(room: Room) { LaunchedEffect(room) { - // Ensure room is attached before entering presence - if (room.status != RoomStatus.Attached) { - room.attach() - } - room.presence.enter( jsonObject { put("status", "Online") } ) } - - // Leave presence when component is disposed - DisposableEffect(room) { - onDispose { - // Note: Consider using a coroutine scope for leave() - // as DisposableEffect doesn't support suspend functions directly - } - } }
🧹 Nitpick comments (5)
src/pages/docs/chat/setup.mdx (1)
202-215: Theandroidandkotlininstantiation snippets are identical.The
androidcode block (lines 202–215) is a verbatim copy of thekotlinblock (lines 187–200). For setup/instantiation this is expected since there's no Compose-specific code, but consider whether showing an identical block under a separate tab adds value or could confuse users.src/pages/docs/chat/rooms/media.mdx (1)
97-121: Severalandroidblocks duplicatekotlinblocks verbatim.For example,
uploadMedia()(lines 97–121) andgetValidMedia()(lines 451–472) are identical to theirkotlincounterparts. The same applies to theremoveMediaFromMessagelogic. Where no Compose-specific code is involved, consider whether both tabs are needed or if theandroidtab could reference thekotlinblock to reduce maintenance burden.src/pages/docs/chat/rooms/message-reactions.mdx (1)
522-550: TheandroidandkotlinMessage/MessageReactionSummary interface blocks are identical.Same observation as other files — this is a type definition block with no Compose-specific code. These identical tabs increase maintenance surface.
src/pages/docs/chat/rooms/messages.mdx (1)
272-274: Androidgetsnippet is identical to the Kotlin snippet — consider whether a separate block is needed.Lines 268–270 (kotlin) and 272–274 (android) contain the exact same code. If the intent is for the android block to demonstrate a Compose-specific pattern (as done in other sections like subscribe/send/update/delete), this block should be updated to show a Compose example. If the API call is the same regardless, this duplication is acceptable but may confuse readers expecting Compose-style examples.
src/pages/docs/chat/rooms/reactions.mdx (1)
97-99: Stale "Jetpack Compose" terminology in<If lang="android">block.The PR objective is to move from "Jetpack" to "Android" annotations to improve LLM comprehension. While "Jetpack Compose" is the official technology name, this text appears inside a block conditioned on
lang="android". For consistency with the migration goals, consider rephrasing to "Android Compose" or simply describing the behavior without the framework name (e.g., "Lifecycle and cleanup are handled automatically when usingLaunchedEffectwithasFlow."). The same pattern occurs intyping.mdx(line 117),presence.mdx(line 119), andoccupancy.mdx(line 150).
There was a problem hiding this comment.
Pull request overview
Updates the Chat documentation and language configuration to replace the legacy jetpack language marker with android, ensuring Jetpack Compose-specific Kotlin snippets render under the Android language selection (DX-553).
Changes:
- Replaced
jetpackwithandroidin Chat MDX<If lang="...">conditions and code fence language tags across setup/rooms/connect/getting-started docs. - Removed the
jetpacklanguage entry from language metadata and updated Android language metadata (including syntax highlighting key). - Updated Chat product language-version mapping to use
androidinstead ofjetpack.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/pages/docs/chat/setup.mdx | Switches Compose/Jetpack-specific blocks from jetpack to android for correct rendering under Android. |
| src/pages/docs/chat/connect.mdx | Updates Compose-related sections/snippets to be keyed under android. |
| src/pages/docs/chat/getting-started/android.mdx | Re-keys fenced snippets to android so the page consistently renders as Android-focused. |
| src/pages/docs/chat/rooms/index.mdx | Replaces jetpack language targeting with android for rooms overview snippets/sections. |
| src/pages/docs/chat/rooms/history.mdx | Updates jetpack sections/snippets to android for history-related Compose guidance. |
| src/pages/docs/chat/rooms/messages.mdx | Re-keys Compose snippets and language-conditional prose from jetpack to android. |
| src/pages/docs/chat/rooms/message-reactions.mdx | Re-keys Compose-specific reactions docs/snippets from jetpack to android. |
| src/pages/docs/chat/rooms/occupancy.mdx | Re-keys Compose occupancy docs/snippets from jetpack to android. |
| src/pages/docs/chat/rooms/presence.mdx | Re-keys Compose presence docs/snippets from jetpack to android. |
| src/pages/docs/chat/rooms/reactions.mdx | Re-keys Compose room reactions docs/snippets from jetpack to android. |
| src/pages/docs/chat/rooms/replies.mdx | Re-keys Compose reply snippets from jetpack to android. |
| src/pages/docs/chat/rooms/typing.mdx | Re-keys Compose typing docs/snippets from jetpack to android. |
| src/pages/docs/chat/rooms/media.mdx | Re-keys Compose media snippets from jetpack to android. |
| src/data/languages/types.ts | Removes jetpack from supported language keys. |
| src/data/languages/languageInfo.ts | Removes jetpack language info; updates android language metadata (syntax highlighting key + alias). |
| src/data/languages/languageData.ts | Updates Chat product language mapping from jetpack to android. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sacOO7
force-pushed
the
fix/android-annotation-for-jetpack
branch
from
2e520e2 to
91b4ffd
Compare
sacOO7
force-pushed
the
fix/android-annotation-for-jetpack
branch
from
3a37fa8 to
9ab8f36
Compare
| 'rest', | ||
| 'css', | ||
| 'laravel', | ||
| 'typescript', |
There was a problem hiding this comment.
typescript here is a duplicate entry
- Replace 'jetpack' language annotation with 'android' across all Chat MDX files - Update languageData.ts, languageInfo.ts, and types.ts for android language - Change android syntaxHighlighterKey from 'java' to 'kotlin'
sacOO7
force-pushed
the
fix/android-annotation-for-jetpack
branch
from
9ab8f36 to
a09e172
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/docs/chat/rooms/presence.mdx (1)
213-242:⚠️ Potential issue | 🟡 Minor
DisposableEffectcleanup is incomplete —leave()is not actually called on dispose.Lines 234–239 show a
DisposableEffectwith anonDisposeblock that only contains a comment suggesting to "consider using a coroutine scope for leave()". This leaves presence cleanup unimplemented. Users following this example won't actually leave presence when the component is disposed.Consider either implementing the cleanup (e.g., using
rememberCoroutineScopeat the composable level) or removing theDisposableEffectblock and adding a note explaining that presence leave is handled via connection lifecycle.Proposed fix: implement leave on dispose
`@Composable` fun EnterPresenceComponent(room: Room) { + val coroutineScope = rememberCoroutineScope() + LaunchedEffect(room) { // Ensure room is attached before entering presence if (room.status != RoomStatus.Attached) { room.attach() } room.presence.enter( jsonObject { put("status", "Online") } ) } // Leave presence when component is disposed DisposableEffect(room) { onDispose { - // Note: Consider using a coroutine scope for leave() - // as DisposableEffect doesn't support suspend functions directly + coroutineScope.launch { + room.presence.leave() + } } } }
🧹 Nitpick comments (4)
src/pages/docs/chat/rooms/message-reactions.mdx (3)
62-68: Android code block is identical to the Kotlin block above — consider adding Compose context.Lines 54–60 (kotlin) and 62–68 (android) contain the exact same snippet. Other android blocks in this file wrap the API call in a
@Composablefunction with coroutine scope. For consistency, consider either adding a Compose wrapper here too, or adding a comment clarifying that the non-Compose usage is identical to Kotlin.
805-811: Same observation: android block duplicates the kotlin block verbatim.Lines 797–803 (kotlin) and 805–811 (android) are identical. The other android snippets in this file provide Jetpack Compose context (e.g.,
@Composable,LaunchedEffect). Consider adding Compose-specific wrapping for consistency, or a brief comment noting the setup is identical.
522-550: Android and Kotlin blocks for message reaction summary structure are identical.Lines 492–520 (kotlin) and 522–550 (android) show the same
MessageReactionSummaryinterface and example. Since this is a data structure description rather than a usage example, duplication is more understandable, but worth noting for consistency with the rest of the file's approach.src/pages/docs/chat/rooms/history.mdx (1)
75-86: Android code block duplicates the Kotlin block verbatim.Both the
kotlin(lines 62–73) andandroid(lines 75–86) code fences contain identical code. If these always stay in sync, consider whether a shared snippet or a combined language annotation could reduce maintenance burden. That said, if the intent is for them to diverge later (e.g., Compose-specific pagination), the duplication is acceptable.
4 participants
Summary by CodeRabbit
Release Notes
Documentation
Chores