-
Notifications
You must be signed in to change notification settings - Fork 2
π :: (#812) λ€νΈμν¬ λͺ¨λΈ νλ μ΄λ² λΆλ¦¬ #813
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
π :: (#812) λ€νΈμν¬ λͺ¨λΈ νλ μ΄λ² λΆλ¦¬ #813
The head ref may contain hidden characters: "refactor/812-\uB124\uD2B8\uC6CC\uD06C-\uBAA8\uB378-\uD50C\uB808\uC774\uBC84-\uBD84\uB9AC"
Conversation
|
Important Review skippedToo many files! 150 files out of 300 files are above the max files limit of 150. You can disable this status message by setting the β¨ 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 |
Summary of ChangesHello @uson1004, 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! μ΄ PRμ λ€νΈμν¬ λ° λ°μ΄ν° λͺ¨λμ λΉλ νλ μ΄λ²λ₯Ό λμ νμ¬ κ°λ° λ° νλ‘λμ νκ²½μ λͺ ννκ² λΆλ¦¬νλ λ° μ€μ μ λ‘λλ€. μ΄λ₯Ό μν΄ κ΄λ²μν νμΌ μ¬κ΅¬μ‘°ν, νλ μ΄λ²λ³ μμ€ μΈνΈ μμ±, κ·Έλ¦¬κ³ UI μ»΄ν¬λνΈ λ° λ·°λͺ¨λΈ λ‘μ§μ κ΄λ ¨ μ λ°μ΄νΈκ° ν¬ν¨λμμ΅λλ€. μ λ°μ μΌλ‘ μ½λλ² μ΄μ€μ λͺ¨λμ±κ³Ό μ μ§λ³΄μμ±μ ν₯μμν€λ κ²μ λͺ©νλ‘ ν©λλ€. 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. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces product flavors (dev/prod) across several core and data modules, including core/jwt, core/network, core/school, data, and network, to support environment-specific configurations. It also refactors package structures by moving UI-related utilities from core.designsystem to core.ui and reorganizing widget-related classes in core/widget. A significant portion of the changes involves adding new data layer components: abstract repository interfaces, their concrete implementations, data models, mappers for network responses, and custom exception classes for various features like authentication, meal, notification, point, and student management. Dependency injection modules using Hilt are also added to bind these new repository implementations. In the app module, the DmsApp.kt file sees UI adjustments, including wrapping the Scaffold in a Box and changing SnackbarHost alignment, along with updating HomeRoute to handle snackbar events. ViewModel logic in HomeViewModel, MealViewModel, and SignInViewModel is streamlined by directly calling repository methods without an explicit runCatching block, as the repository methods already return Result. Review comments highlight two main issues: the AuthRepositoryImpl's incomplete token handling when autoSignIn is false, where tokens are discarded instead of being maintained in memory for the app session, and the NotificationRepositoryImpl having TODO implementations for registerDeviceNotificationToken and cancelDeviceTokenRegistration, indicating missing core push notification functionality.
| // FIXME: λ§μ½ μλ λ‘κ·ΈμΈμ νμ§ μμ μνμμ ν ν° μ¬λ°κΈμ μ΄λ»κ² μ²λ¦¬νλκ°? | ||
| if (autoSignIn) { | ||
| val tokens = signInResponse.extractTokens() | ||
| val features = signInResponse.extractFeatures() | ||
| jwtProvider.updateTokens(tokens) | ||
| schoolProvider.updateFeatures(features) | ||
| } |
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.
FIXME μ£Όμμμ μ§μ λ λ°μ κ°μ΄, autoSignInμ΄ falseμΌ κ²½μ°μ ν ν° μ²λ¦¬ λ‘μ§μ΄ λΆμμ ν©λλ€. νμ¬ μ½λλ autoSignInμ΄ trueμΌ λλ§ ν ν°μ μ μ₯νκ³ μμΌλ©°, falseμΌ κ²½μ°μλ λ‘κ·ΈμΈ μ λ°κΈλ ν ν°μ μ¬μ©νμ§ μκ³ λ²λ¦¬κ³ μμ΅λλ€. μλ λ‘κ·ΈμΈμ μ¬μ©νμ§ μλλΌλ μ± μΈμ
λμμλ ν ν°μ λ©λͺ¨λ¦¬μ μ μ§νμ¬ API νΈμΆμ μ¬μ©ν΄μΌ ν©λλ€. μ΄ λΆλΆμ λͺ
νν ꡬνν΄μΌ ν©λλ€.
| override suspend fun registerDeviceNotificationToken(deviceToken: String): Result<Unit> = runCatching { | ||
| TODO("Not yet implemented") | ||
| } | ||
|
|
||
| override suspend fun cancelDeviceTokenRegistration(deviceToken: String): Result<Unit> = runCatching { | ||
| TODO("Not yet implemented") | ||
| } |
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.
κ°μ
μμ μ¬ν
μΆκ° λ‘ ν λ§