Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces comprehensive support for a 'live status' feature within user profiles, focusing on the underlying data layer. It establishes new data models to represent profile status, integrates these models into the application's local persistence layer, and provides the necessary repository functions to update and manage a user's live status, including going live with an external stream URL or ending a live session. The changes ensure that profile status information can be effectively stored, retrieved, and synchronized across the application. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces support for profile live status across the data layer. The changes are well-structured, including updates to the domain models, repository, database schema, and write queue. The implementation is comprehensive and follows existing patterns in the codebase. I have one suggestion to improve the feature by avoiding hardcoded empty strings for the live stream's title and description, which would provide a richer user experience.
| external = ExternalExternal( | ||
| uri = LexiconUri(update.streamUrl), | ||
| title = "", | ||
| description = "", | ||
| ), |
There was a problem hiding this comment.
The title and description for the live status are hardcoded as empty strings. This is also repeated in the toOutcome block on lines 567-568.
This is a missed opportunity to provide more context about the live stream. Consider enhancing the Profile.StatusUpdate.GoLive model to accept an optional title and description. If they aren't provided, you could attempt to fetch this metadata from the streamUrl (e.g., by parsing OpenGraph tags from the HTML at the URL). This would make the feature more robust and provide a better user experience.
| "formatVersion": 1, | ||
| "database": { | ||
| "version": 34, | ||
| "identityHash": "1e975be4633a2c929f918f6201d3bdf4", |
There was a problem hiding this comment.
If the DB identity hash changes, it means you're missing a migration. The identity has hash for a DB version shouldn't change.
| import com.tunjid.heron.data.core.types.ProfileId | ||
| import kotlin.time.Instant | ||
|
|
||
| @Entity( |
There was a problem hiding this comment.
I don't think creating a ProfileStatusEntity is needed. A profile can only have one status, and that status is the one with the self record key. Each time a profile goes live, its always updating its self status.
That is, a profile status is a 1:1 record. The status should be a nullable embedded record in the ProfileEntity
| ) | ||
|
|
||
| @Serializable | ||
| data class ProfileStatus( |
There was a problem hiding this comment.
Since this is most likely going to be an embedded model in ProfileEntity, we should make it a flat data class.
There was a problem hiding this comment.
i.e:
data class ProfileStatus(
val uri: String? = null,
val status: String,
val uri: String,
val title: String,
val description: String,
val thumb: ImageUri? = null,
val expiresAt: Instant? = null,
val isActive: Boolean? = null,
val isDisabled: Boolean? = null,
)
No description provided.