-
Notifications
You must be signed in to change notification settings - Fork 308
Address Lint findings #6058
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Address Lint findings #6058
Conversation
SDK Size Comparison 📏
|
|
WalkthroughThe PR applies lint configuration updates across multiple build files to suppress translation-related warnings, optimizes Compose reactive flow collection by introducing remember-wrapped caching to improve recomposition behavior, extends navigation with MENTIONS and THREADS tabs, converts a Composable function to a regular function, and corrects invalid color component values. Changes
Poem
Estimated code review effort🎯 2 (Simple) | ⏱️ ~20 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/list/adapter/internal/MessageListItemDiffCallback.kt (1)
30-48: Add documentation comment for the lint suppression.The
@SuppressLint("DiffUtilEquals")suppression lacks explanation, which violates the coding guideline to document suppressions. However, the suppression is justified: all compared types—ThreadSeparatorItem,StartOfTheChannelItem,Channel, andDate—are either Kotlin data classes (which auto-generate correctequals()implementations) or standard Java types with proper equality checks.Add a brief comment explaining why the suppression is safe:
// DiffUtilEquals suppressed: All compared types have correct equals() implementations. // ThreadSeparatorItem and StartOfTheChannelItem are data classes; Channel is a data class; // Date is java.util.Date with standard equals(). @SuppressLint("DiffUtilEquals") override fun areContentsTheSame(oldItem: MessageListItem, newItem: MessageListItem): Boolean {
🧹 Nitpick comments (2)
stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/component/MessageInfoComponentFactory.kt (1)
269-287: Consider creating separate Date instances for preview timestamps.The same
sentDateobject is mutated multiple times viaapply, causing all timestamps to accumulate incorrectly. Eachapplycall modifies and returns the same object, so by line 287,sentDatehas 6 hours added cumulatively rather than the intended distinct offsets.🔎 Proposed fix
val reads = listOf( ChannelUserRead( user = user1, lastReceivedEventDate = Date(), unreadMessages = 0, - lastRead = sentDate.apply { time += 2.hours.inWholeMilliseconds }, + lastRead = Date(sentDate.time + 2.hours.inWholeMilliseconds), lastReadMessageId = null, ), ChannelUserRead( user = user2, lastReceivedEventDate = Date(), unreadMessages = 0, - lastRead = sentDate.apply { time += 3.hours.inWholeMilliseconds }, + lastRead = Date(sentDate.time + 3.hours.inWholeMilliseconds), lastReadMessageId = null, ), ) val deliveredReads = listOf( ChannelUserRead( user = user3, lastReceivedEventDate = Date(), unreadMessages = 0, lastRead = Date(), lastReadMessageId = null, - lastDeliveredAt = sentDate.apply { time += 1.hours.inWholeMilliseconds }, + lastDeliveredAt = Date(sentDate.time + 1.hours.inWholeMilliseconds), lastDeliveredMessageId = null, ), )stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/feature/channel/list/ChannelsActivity.kt (1)
142-144: Different caching approach: activity-level flows vs. remember-wrapped flows.These flows are created at the activity level in
onCreate(lines 142-144) and collected inside the composable (lines 155-156), which is valid and ensures flows are created once per activity instance. However, this differs from the pattern inChatsActivity.ktwhere flows are created inside composables usingrememberblocks.While both approaches work correctly, consider standardizing on one pattern across the codebase for consistency. The
remember-based approach (as in ChatsActivity) is more idiomatic for Compose as it ties flow lifecycle to composition rather than activity.💡 Optional refactor to align with ChatsActivity pattern
Move flow creation inside the composable with
remember:override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) - val globalStateFlow = ChatClient.instance().globalStateFlow - val unreadChannelsCountFlow = globalStateFlow.flatMapLatest { it.channelUnreadCount } - val unreadThreadsCountFlow = globalStateFlow.flatMapLatest { it.unreadThreadsCount } - setContent { var selectedTab by rememberSaveable { mutableStateOf(AppBottomBarOption.CHATS) } + val globalStateFlow = remember { ChatClient.instance().globalStateFlow } + val unreadChannelsCount by remember { globalStateFlow.flatMapLatest { it.channelUnreadCount } } + .collectAsStateWithLifecycle(0) + val unreadThreadsCount by remember { globalStateFlow.flatMapLatest { it.unreadThreadsCount } } + .collectAsStateWithLifecycle(0) - val unreadChannelsCount by unreadChannelsCountFlow.collectAsStateWithLifecycle(0) - val unreadThreadsCount by unreadThreadsCountFlow.collectAsStateWithLifecycle(0)Also applies to: 155-156
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
stream-chat-android-client/build.gradle.ktsstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/feature/channel/list/ChannelsActivity.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/chats/ChatsActivity.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/component/MessageInfoComponentFactory.ktstream-chat-android-compose/build.gradle.ktsstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/util/MessageUtils.ktstream-chat-android-ui-common/build.gradle.ktsstream-chat-android-ui-components/build.gradle.ktsstream-chat-android-ui-components/lint-baseline.xmlstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/list/adapter/internal/MessageListItemDiffCallback.ktstream-chat-android-ui-guides/src/main/java/io/getstream/chat/android/guides/catalog/compose/customizingimageandvideoattachments/factory/MultimediaAttachmentFactory.kt
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{kt,kts}
📄 CodeRabbit inference engine (AGENTS.md)
Format and apply Kotlin style with Spotless (4 spaces, no wildcard imports, licence headers)
Files:
stream-chat-android-client/build.gradle.ktsstream-chat-android-compose/build.gradle.ktsstream-chat-android-ui-common/build.gradle.ktsstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/list/adapter/internal/MessageListItemDiffCallback.ktstream-chat-android-ui-components/build.gradle.ktsstream-chat-android-ui-guides/src/main/java/io/getstream/chat/android/guides/catalog/compose/customizingimageandvideoattachments/factory/MultimediaAttachmentFactory.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/util/MessageUtils.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/feature/channel/list/ChannelsActivity.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/chats/ChatsActivity.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/component/MessageInfoComponentFactory.kt
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Use@OptInannotations explicitly; avoid suppressions unless documented
Document public APIs with KDoc, including thread expectations and state notes
Files:
stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/list/adapter/internal/MessageListItemDiffCallback.ktstream-chat-android-ui-guides/src/main/java/io/getstream/chat/android/guides/catalog/compose/customizingimageandvideoattachments/factory/MultimediaAttachmentFactory.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/util/MessageUtils.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/feature/channel/list/ChannelsActivity.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/chats/ChatsActivity.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/component/MessageInfoComponentFactory.kt
**/stream-chat-android-compose/**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/stream-chat-android-compose/**/*.kt: Compose components should follow noun-based naming (e.g.,MessageList,ChannelListHeader)
Compose previews should use@StreamPreviewhelpers
Files:
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/util/MessageUtils.kt
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/stream-chat-android-compose/**/*.kt : Compose previews should use `StreamPreview` helpers
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/stream-chat-android-compose/**/*Test.kt : Add Paparazzi snapshots for Compose UI regressions and run `verifyPaparazziDebug`
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/stream-chat-android-ui-components/**/*Test.kt : Record Shot baselines when behaviour changes in XML kit UI tests
Applied to files:
stream-chat-android-client/build.gradle.ktsstream-chat-android-compose/build.gradle.ktsstream-chat-android-ui-common/build.gradle.ktsstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/list/adapter/internal/MessageListItemDiffCallback.ktstream-chat-android-ui-components/build.gradle.ktsstream-chat-android-ui-guides/src/main/java/io/getstream/chat/android/guides/catalog/compose/customizingimageandvideoattachments/factory/MultimediaAttachmentFactory.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/feature/channel/list/ChannelsActivity.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/chats/ChatsActivity.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/component/MessageInfoComponentFactory.kt
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/stream-chat-android-compose/**/*Test.kt : Add Paparazzi snapshots for Compose UI regressions and run `verifyPaparazziDebug`
Applied to files:
stream-chat-android-compose/build.gradle.ktsstream-chat-android-ui-components/build.gradle.ktsstream-chat-android-ui-guides/src/main/java/io/getstream/chat/android/guides/catalog/compose/customizingimageandvideoattachments/factory/MultimediaAttachmentFactory.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/util/MessageUtils.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/chats/ChatsActivity.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/component/MessageInfoComponentFactory.kt
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/stream-chat-android-compose/**/*.kt : Compose previews should use `StreamPreview` helpers
Applied to files:
stream-chat-android-compose/build.gradle.ktsstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/util/MessageUtils.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/feature/channel/list/ChannelsActivity.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/chats/ChatsActivity.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/component/MessageInfoComponentFactory.kt
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/stream-chat-android-compose/**/*.kt : Compose components should follow noun-based naming (e.g., `MessageList`, `ChannelListHeader`)
Applied to files:
stream-chat-android-compose/build.gradle.ktsstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/util/MessageUtils.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/feature/channel/list/ChannelsActivity.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/chats/ChatsActivity.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/component/MessageInfoComponentFactory.kt
🔇 Additional comments (11)
stream-chat-android-ui-guides/src/main/java/io/getstream/chat/android/guides/catalog/compose/customizingimageandvideoattachments/factory/MultimediaAttachmentFactory.kt (2)
43-43: LGTM! Lint correctly caught invalid RGB color values.The color component values have been corrected from 256 to 255, which is the maximum valid value for 8-bit RGB channels (valid range: 0-255).
58-58: LGTM! Consistent fix applied.Same correction as Line 43, ensuring both play button overlays use valid color component values.
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/util/MessageUtils.kt (1)
40-44: LGTM! Proper use ofrememberto cache the flow.Wrapping the flow transformation in
remember(cid, messageId)prevents unnecessary Flow recreation on every recomposition while maintaining correct invalidation when the keys change. This aligns with Compose best practices for collecting flows.stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/component/MessageInfoComponentFactory.kt (1)
125-125: LGTM! Proper use ofrememberto optimize flow collection.The
remember(message)wrapper correctly prevents the flow from being recreated on every recomposition, caching it per message instance. This optimization aligns with Compose best practices and addresses the lint findings mentioned in the PR objectives.stream-chat-android-client/build.gradle.kts (1)
43-46: LGTM - Lint configuration updated as intended.The addition of the MissingTranslation suppression aligns with the PR objectives to address lint findings by selectively disabling checks.
stream-chat-android-compose/build.gradle.kts (1)
28-31: LGTM - Lint configuration added as intended.The lint block correctly disables translation-related checks, aligning with the PR's goal to address lint findings.
stream-chat-android-ui-common/build.gradle.kts (1)
24-26: LGTM - Lint configuration added as intended.The MissingTranslation suppression is correctly configured and aligns with the PR objectives.
stream-chat-android-ui-components/build.gradle.kts (1)
23-27: LGTM - Lint configuration with baseline properly configured.The lint configuration correctly disables translation checks and references the baseline file at
stream-chat-android-ui-components/lint-baseline.xml, which exists and aligns with the PR objectives of introducing a lint baseline for the XML components module. The Gradle DSL syntax and formatting are correct.stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/chats/ChatsActivity.kt (1)
239-243: LGTM! Correct use ofrememberfor flow caching.The
rememberblocks properly cache theglobalStateFlowreference and the derived flow transformations, preventing unnecessary recomputations on every recomposition. This follows Compose best practices for optimizing reactive flow collection.stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/feature/channel/list/ChannelsActivity.kt (1)
223-225: LGTM! Proper extension of bottom navigation.The new content branches for
MENTIONSandTHREADScorrectly wire up the corresponding composables (MentionsContentandThreadsContent) defined later in the file.stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/list/adapter/internal/MessageListItemDiffCallback.kt (1)
19-19: LGTM: Necessary import for lint suppression.The import is required for the
@SuppressLintannotation used below.



🎯 Goal
While working on the common CI, I realized we are not running Lint only in Chat and that it yields many findings. I'm addressing the findings here, while we'll start running Lint in CI in #6050
🛠 Implementation details
rememberwhen we should)🎨 UI Changes
None
🧪 Testing
The only production change is the
rememberinshowOriginalTextAsState. To test it, you have to:it&en)showOriginalTranslationEnabled = truetoChatThemeinMessagesActivityituser)☑️Contributor Checklist
General
Code & documentation
☑️Reviewer Checklist
🎉 GIF
Please provide a suitable gif that describes your work on this pull request
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.