refactor: feature 모듈간 의존성 제거 #39
Conversation
screens 모듈을 생성하여, feature 모듈들이 screens 모듈만을 바라보도록 설계 변경
# Conflicts: # feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginPresenter.kt # feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginScreen.kt
|
""" Walkthrough이 변경 사항은 새로운 Changes
Sequence Diagram(s)sequenceDiagram
participant FeatureModule as Feature Module (예: Home, Login)
participant Screens as :screens 모듈
participant Navigation as Navigation System
FeatureModule->>Screens: Screen 객체 import 및 사용
FeatureModule->>Navigation: Navigation(Screen)
Navigation->>Screens: Screen 타입 확인 및 라우팅
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(해당 사항 없음) Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/component/MainBottomBar.kt (1)
160-170: lambda 내부 변수 이름 충돌 경계
getCurrentTab()에서return currentScreen?.let { screen -> MainTab.entries.find { it.screen::class == currentScreen::class } }
let의 파라미터명을screen으로 지정하면서 바깥 scope의currentScreen을 다시 참조하고 있습니다. 기능상 문제는 없지만 가독성이 떨어집니다.-return currentScreen?.let { screen -> - MainTab.entries.find { it.screen::class == currentScreen::class } +return currentScreen?.let { current -> + MainTab.entries.find { it.screen::class == current::class } }처럼 파라미터를 활용하거나,
it사용을 고려하면 혼동을 줄일 수 있습니다.feature/login/src/main/kotlin/com/ninecraft/booket/feature/termsagreement/TermsAgreementPresenter.kt (1)
53-55: TODO 주석에 대한 향후 계획 확인이 필요합니다.웹뷰 화면으로의 이동이 구현되지 않은 상태입니다. 이전 학습 내용에 따르면 약관 상세 URL이 아직 결정되지 않아 의도적으로 비어있는 상태인 것으로 보입니다.
약관 상세 URL이 결정되면 해당 기능을 구현할 수 있도록 도움을 드릴 수 있습니다. 별도 이슈로 추적하시겠습니까?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
build-logic/src/main/kotlin/AndroidFeatureConventionPlugin.kt(2 hunks)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt(2 hunks)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeScreen.kt(3 hunks)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUiState.kt(1 hunks)feature/library/build.gradle.kts(0 hunks)feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/HandleLibrarySideEffects.kt(1 hunks)feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt(6 hunks)feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryScreen.kt(4 hunks)feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryUiState.kt(1 hunks)feature/login/build.gradle.kts(0 hunks)feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/HandleLoginSideEffects.kt(1 hunks)feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginPresenter.kt(4 hunks)feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginScreen.kt(3 hunks)feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginUiState.kt(1 hunks)feature/login/src/main/kotlin/com/ninecraft/booket/feature/termsagreement/TermsAgreementPresenter.kt(3 hunks)feature/login/src/main/kotlin/com/ninecraft/booket/feature/termsagreement/TermsAgreementScreen.kt(6 hunks)feature/login/src/main/kotlin/com/ninecraft/booket/feature/termsagreement/TermsAgreementUiState.kt(1 hunks)feature/main/build.gradle.kts(0 hunks)feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/MainActivity.kt(1 hunks)feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/component/MainBottomBar.kt(1 hunks)feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/component/MainTab.kt(1 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt(2 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchScreen.kt(3 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchUiState.kt(1 hunks)gradle/libs.versions.toml(1 hunks)screens/.gitignore(1 hunks)screens/build.gradle.kts(1 hunks)screens/src/main/kotlin/com/ninecraft/booket/screens/Screens.kt(1 hunks)settings.gradle.kts(1 hunks)
💤 Files with no reviewable changes (3)
- feature/login/build.gradle.kts
- feature/library/build.gradle.kts
- feature/main/build.gradle.kts
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#35
File: feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/TermsAgreementScreen.kt:127-127
Timestamp: 2025-07-09T01:14:29.836Z
Learning: In the Reed-Android project's TermsAgreementScreen.kt, the OnTermDetailClick event is intentionally passed an empty string for the URL parameter because the actual URLs for terms detail pages haven't been decided yet. This is a temporary implementation that will be updated once the URLs are finalized.
feature/login/src/main/kotlin/com/ninecraft/booket/feature/termsagreement/TermsAgreementUiState.kt (1)
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#35
File: feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/TermsAgreementScreen.kt:127-127
Timestamp: 2025-07-09T01:14:29.836Z
Learning: In the Reed-Android project's TermsAgreementScreen.kt, the OnTermDetailClick event is intentionally passed an empty string for the URL parameter because the actual URLs for terms detail pages haven't been decided yet. This is a temporary implementation that will be updated once the URLs are finalized.
feature/login/src/main/kotlin/com/ninecraft/booket/feature/termsagreement/TermsAgreementPresenter.kt (1)
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#35
File: feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/TermsAgreementScreen.kt:127-127
Timestamp: 2025-07-09T01:14:29.836Z
Learning: In the Reed-Android project's TermsAgreementScreen.kt, the OnTermDetailClick event is intentionally passed an empty string for the URL parameter because the actual URLs for terms detail pages haven't been decided yet. This is a temporary implementation that will be updated once the URLs are finalized.
feature/login/src/main/kotlin/com/ninecraft/booket/feature/termsagreement/TermsAgreementScreen.kt (1)
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#35
File: feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/TermsAgreementScreen.kt:127-127
Timestamp: 2025-07-09T01:14:29.836Z
Learning: In the Reed-Android project's TermsAgreementScreen.kt, the OnTermDetailClick event is intentionally passed an empty string for the URL parameter because the actual URLs for terms detail pages haven't been decided yet. This is a temporary implementation that will be updated once the URLs are finalized.
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryScreen.kt (1)
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#35
File: feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/TermsAgreementScreen.kt:127-127
Timestamp: 2025-07-09T01:14:29.836Z
Learning: In the Reed-Android project's TermsAgreementScreen.kt, the OnTermDetailClick event is intentionally passed an empty string for the URL parameter because the actual URLs for terms detail pages haven't been decided yet. This is a temporary implementation that will be updated once the URLs are finalized.
feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginScreen.kt (2)
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#32
File: core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/button/ButtonColorStyle.kt:10-16
Timestamp: 2025-07-08T12:33:01.863Z
Learning: Reed Android 프로젝트에서 KAKAO 버튼 스타일은 디자이너가 pressed 상태 색상을 별도로 정의하지 않았기 때문에 pressed 상태에서도 동일한 Kakao 색상을 사용한다.
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#35
File: feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/TermsAgreementScreen.kt:127-127
Timestamp: 2025-07-09T01:14:29.836Z
Learning: In the Reed-Android project's TermsAgreementScreen.kt, the OnTermDetailClick event is intentionally passed an empty string for the URL parameter because the actual URLs for terms detail pages haven't been decided yet. This is a temporary implementation that will be updated once the URLs are finalized.
screens/src/main/kotlin/com/ninecraft/booket/screens/Screens.kt (1)
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#35
File: feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/TermsAgreementScreen.kt:127-127
Timestamp: 2025-07-09T01:14:29.836Z
Learning: In the Reed-Android project's TermsAgreementScreen.kt, the OnTermDetailClick event is intentionally passed an empty string for the URL parameter because the actual URLs for terms detail pages haven't been decided yet. This is a temporary implementation that will be updated once the URLs are finalized.
🧬 Code Graph Analysis (2)
build-logic/src/main/kotlin/AndroidFeatureConventionPlugin.kt (1)
build-logic/src/main/kotlin/com/ninecraft/booket/convention/Dependencies.kt (2)
implementation(6-8)project(42-52)
screens/build.gradle.kts (1)
build-logic/src/main/kotlin/com/ninecraft/booket/convention/Dependencies.kt (2)
implementation(6-8)api(10-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (47)
settings.gradle.kts (1)
42-44:":screens"모듈 추가 👍신규 모듈을
include목록에 추가한 변경이 빌드 구성을 깨뜨릴 위험은 없어 보이며, 이번 PR 의존성 분리 목적과 잘 맞습니다.
추가적인 설정 누락(예:gradle.properties의android.defaults.buildfeatures.buildconfig=false)은 없는지 한 번만 확인해 주세요.feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/MainActivity.kt (1)
13-15: 의존성 정리 여부 확인 필요
LoginScreenimport 경로가screens모듈로 변경되었습니다.
feature:main모듈의build.gradle.kts에서 더 이상feature:login에 대한 종속성이 남아있지 않은지 확인해 주세요. 남아있다면 제거하지 않으면 중복·불필요 의존성이 생길 수 있습니다.feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/component/MainTab.kt (1)
6-9: 스크린 모듈로의 경로 변경 확인 완료
HomeScreen,LibraryScreen,SearchScreenimport 경로가 일관성 있게screens모듈로 교체되었습니다. 해당 모듈이Parcelable및Circuit등록을 모두 갖추고 있다면 문제없이 동작할 것으로 보입니다.screens/.gitignore (1)
1-1: 빌드 산출물 무시 규칙 추가 확인
/build디렉터리 제외 규칙 추가 👍
모듈 단위.gitignore관리가 깔끔합니다.screens/build.gradle.kts (1)
13-15: 의존성 설정 검토 완료screens 모듈의 의존성 설정이 적절합니다:
core.model은 Screen 객체가 모델 타입을 참조할 수 있도록 implementation으로 설정circuit.runtime은 다른 모듈에서 Circuit 타입을 사용할 수 있도록 api로 설정Circuit 런타임을 api로 노출하는 것은 feature 모듈들이 CircuitUiState와 CircuitUiEvent를 직접 사용할 수 있도록 하는 좋은 설계입니다.
gradle/libs.versions.toml (1)
107-107: circuit-runtime 라이브러리 정의 적절함circuit-runtime 라이브러리가 기존 circuit 버전을 참조하여 일관성 있게 정의되었습니다. 이는 screens 모듈에서 CircuitUiState와 CircuitUiEvent를 사용하기 위한 필수 의존성입니다.
build-logic/src/main/kotlin/AndroidFeatureConventionPlugin.kt (2)
1-7: import 순서 정리 완료import 문이 알파벳 순으로 정리되어 가독성이 향상되었습니다. 이는 코드 일관성 유지에 도움이 됩니다.
27-27: screens 모듈 의존성 추가 적절함모든 feature 모듈이 screens 모듈에 의존하도록 설정하여 순환 의존성 문제를 해결하는 좋은 접근입니다. 이제 각 feature 모듈은 screens 모듈의 Screen 객체들을 직접 참조할 수 있습니다.
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt (1)
5-5: 리팩토링이 올바르게 적용되었습니다screens 모듈에서 SearchScreen을 import하고 SearchUiState 타입으로 일관되게 변경하여 feature 모듈 간 의존성을 성공적으로 제거했습니다.
Also applies to: 17-17, 20-20, 23-23
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt (1)
5-5: 일관된 리팩토링 패턴이 적용되었습니다SearchPresenter와 동일한 패턴으로 HomeUiState 타입 변경과 screens 모듈 import가 올바르게 적용되었습니다.
Also applies to: 17-17, 20-20, 23-23
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/HandleLibrarySideEffects.kt (1)
10-11: 사이드 이펙트 처리 로직이 새로운 타입으로 올바르게 변경되었습니다LibraryUiState, LibraryUiEvent, LibrarySideEffect 타입으로의 변경이 일관되게 적용되었고, 기존 로직이 올바르게 유지되었습니다.
Also applies to: 17-17, 25-25
feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/HandleLoginSideEffects.kt (1)
11-12: 로그인 사이드 이펙트 처리가 새로운 타입으로 올바르게 변경되었습니다LoginUiState, LoginUiEvent, LoginSideEffect 타입으로의 변경이 일관되게 적용되었고, 카카오 로그인 및 에러 처리 로직이 올바르게 유지되었습니다.
Also applies to: 19-19, 23-23, 26-26, 31-31, 39-39
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt (2)
11-12: Presenter 클래스가 새로운 아키텍처에 맞게 올바르게 변경되었습니다screens 모듈에서 Screen 객체들을 import하고 LibraryUiState 타입으로 일관되게 변경하여 feature 모듈 간 의존성을 성공적으로 제거했습니다.
Also applies to: 29-29, 32-32, 35-35
51-51: 이벤트 처리와 사이드 이펙트 로직이 새로운 타입으로 올바르게 변경되었습니다LibraryUiEvent, LibrarySideEffect 타입으로의 변경이 일관되게 적용되었고, 사용자 프로필 조회 및 로그아웃 로직이 올바르게 유지되었습니다.
Also applies to: 73-73, 75-75, 79-79, 91-91, 111-111
feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginPresenter.kt (2)
9-10: 중앙화된 screens 모듈 사용 승인screens 모듈에서 LoginScreen과 TermsAgreementScreen을 import하는 것이 feature 모듈 간 순환 의존성을 해결하는 올바른 접근 방식입니다.
25-75: UI state 및 event 모델 리팩토링 승인LoginPresenter가 새로운 UI state 모델(LoginUiState, LoginSideEffect, LoginUiEvent)을 올바르게 사용하도록 업데이트되었습니다. 기존 로직은 유지하면서 타입 참조만 변경하여 안전한 리팩토링이 이루어졌습니다.
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryUiState.kt (1)
1-21: 새로운 UI state 모델 정의 승인LibraryUiState, LibrarySideEffect, LibraryUiEvent가 올바르게 정의되었습니다. Circuit 런타임의 인터페이스들을 적절히 구현하고 있으며, 타입 안전성과 구조 일관성이 보장되었습니다. 이는 feature 모듈의 UI state 관리를 표준화하는 좋은 접근 방식입니다.
feature/login/src/main/kotlin/com/ninecraft/booket/feature/termsagreement/TermsAgreementUiState.kt (2)
7-11: ImmutableList 사용으로 상태 불변성 보장 승인TermsAgreementUiState에서 ImmutableList를 사용하여 약관 동의 상태의 불변성을 보장하는 것이 좋은 설계입니다. 이는 상태 변경 시 예기치 않은 부작용을 방지하고 안전한 상태 관리를 가능하게 합니다.
13-19: 포괄적인 이벤트 정의 승인TermsAgreementUiEvent가 약관 동의 화면에서 발생할 수 있는 모든 사용자 상호작용을 포괄적으로 정의하고 있습니다. 전체 약관 동의, 개별 약관 클릭, 뒤로가기, 약관 상세보기, 시작 버튼 클릭 등 필요한 모든 이벤트가 포함되어 있습니다.
feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginScreen.kt (2)
25-25: 중앙화된 screens 모듈 사용 승인LoginScreen을 중앙화된 screens 모듈에서 import하는 것이 feature 모듈 간 의존성을 제거하는 올바른 접근 방식입니다.
33-91: UI state 타입 마이그레이션 승인Login 컴포저블과 프리뷰가 새로운 LoginUiState 타입을 올바르게 사용하도록 업데이트되었습니다. 이벤트 디스패치도 LoginUiEvent로 일관성 있게 변경되었으며, 컴포저블의 기존 로직은 안전하게 유지되었습니다.
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchScreen.kt (2)
12-12: 중앙화된 screens 모듈 사용 승인SearchScreen을 중앙화된 screens 모듈에서 import하는 것이 feature 모듈 간 의존성을 제거하는 올바른 접근 방식입니다.
19-51: UI state 타입 마이그레이션 승인Search와 SearchContent 컴포저블이 새로운 SearchUiState 타입을 올바르게 사용하도록 업데이트되었습니다. 프리뷰도 새로운 UI state 타입으로 일관성 있게 변경되었으며, 다른 feature 모듈들과 동일한 리팩토링 패턴을 따르고 있어 코드 일관성이 향상되었습니다.
feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginUiState.kt (3)
6-10: UI 상태 구조가 잘 정의되어 있습니다.LoginUiState가 CircuitUiState를 올바르게 구현하고 있으며, 로딩 상태, 사이드 이펙트, 이벤트 싱크를 적절히 포함하고 있습니다. 기본값 설정도 합리적입니다.
12-15: 사이드 이펙트 정의가 명확합니다.KakaoLogin과 ShowToast 사이드 이펙트가 sealed interface로 적절히 정의되어 있습니다. 타입 안전성을 보장하면서도 확장 가능한 구조입니다.
17-22: 이벤트 정의가 체계적입니다.LoginUiEvent의 각 이벤트가 명확한 의미를 가지고 있으며, 로그인 플로우의 모든 단계를 적절히 커버하고 있습니다. CircuitUiEvent 구현도 올바릅니다.
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryScreen.kt (4)
23-23: screens 모듈에서 LibraryScreen 임포트가 올바릅니다.중앙화된 screens 모듈에서 LibraryScreen을 임포트하여 모듈 간 의존성을 성공적으로 해결했습니다.
30-30: LibraryUiState 사용으로 전환이 적절합니다.Library와 LibraryContent 컴포저블 함수가 외부 정의된 LibraryUiState를 사용하도록 올바르게 변경되었습니다. 이는 UI 상태와 스크린 정의를 분리하는 새로운 아키텍처와 일치합니다.
Also applies to: 52-52
74-74: 이벤트 처리 방식이 일관되게 변경되었습니다.LibraryUiEvent.OnLogoutButtonClick 사용으로 변경되어 새로운 이벤트 처리 패턴을 올바르게 따르고 있습니다.
98-102: 프리뷰 함수가 새로운 상태 구조에 맞게 업데이트되었습니다.LibraryUiState를 사용하여 프리뷰를 생성하도록 올바르게 변경되었습니다.
feature/login/src/main/kotlin/com/ninecraft/booket/feature/termsagreement/TermsAgreementScreen.kt (5)
1-1: 패키지 구조 변경이 적절합니다.feature.termsagreement 패키지로 변경되어 로그인 관련 기능의 모듈화가 개선되었습니다.
36-36: screens 모듈에서 TermsAgreementScreen 임포트가 올바릅니다.중앙화된 screens 모듈에서 TermsAgreementScreen을 임포트하여 모듈 간 순환 의존성을 성공적으로 해결했습니다.
44-44: TermsAgreementUiState 사용으로 전환이 적절합니다.외부 정의된 TermsAgreementUiState를 사용하여 UI 상태와 스크린 정의가 성공적으로 분리되었습니다.
54-54: 이벤트 처리 방식이 일관되게 변경되었습니다.모든 이벤트 처리가 TermsAgreementUiEvent를 사용하도록 올바르게 변경되어 새로운 아키텍처 패턴을 따르고 있습니다.
Also applies to: 86-86, 105-105, 115-115
108-108: 빈 문자열 URL 사용은 의도적인 구현입니다.이전 학습에서 확인된 바와 같이, 약관 상세 페이지 URL이 아직 결정되지 않았기 때문에 빈 문자열을 전달하는 것이 의도적인 임시 구현입니다.
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeScreen.kt (3)
12-12: screens 모듈에서 HomeScreen 임포트가 올바릅니다.중앙화된 screens 모듈에서 HomeScreen을 임포트하여 모듈 간 의존성 문제를 성공적으로 해결했습니다.
19-19: HomeUiState 사용으로 전환이 적절합니다.Home과 HomeContent 컴포저블 함수가 외부 정의된 HomeUiState를 사용하도록 올바르게 변경되어 새로운 아키텍처와 일치합니다.
Also applies to: 37-37
48-50: 프리뷰 함수가 새로운 상태 구조에 맞게 업데이트되었습니다.HomeUiState를 사용하여 프리뷰를 생성하도록 올바르게 변경되었습니다.
screens/src/main/kotlin/com/ninecraft/booket/screens/Screens.kt (3)
6-8: ReedScreen 추상 클래스 설계가 우수합니다.Screen 인터페이스를 구현하면서 name 속성과 toString() 메서드를 제공하는 깔끔한 추상화입니다. 일관된 스크린 표현을 위한 좋은 기반 클래스입니다.
10-23: 스크린 객체 정의가 일관되고 체계적입니다.모든 스크린 객체가 data object로 정의되어 싱글톤 패턴을 보장하며, @parcelize 어노테이션으로 직렬화를 지원합니다. 네이밍 패턴도 일관되게 "ScreenName()" 형태로 통일되어 있습니다.
1-24: 모듈 간 순환 의존성 해결 방안이 효과적입니다.새로운 screens 모듈을 도입하여 각 feature 모듈이 screens 모듈만을 의존하도록 함으로써 순환 의존성 문제를 성공적으로 해결했습니다. tivi 프로젝트의 접근 방식을 참고한 것도 좋은 선택입니다.
feature/login/src/main/kotlin/com/ninecraft/booket/feature/termsagreement/TermsAgreementPresenter.kt (6)
1-1: 패키지 분리가 올바르게 적용되었습니다.login 모듈에서 termsagreement로 패키지가 분리되어 모듈 간 의존성 제거 목표에 부합합니다.
9-10: screens 모듈에서 Screen 객체를 올바르게 import하고 있습니다.중앙집중식 screen 정의를 사용하여 feature 모듈 간 순환 의존성 문제를 해결하는 좋은 접근 방식입니다.
24-24: UI 상태 타입이 올바르게 업데이트되었습니다.
TermsAgreementScreen.State에서TermsAgreementUiState로 변경되어 중첩 클래스 의존성이 제거되었습니다.
38-61: 이벤트 처리 로직이 새로운 아키텍처에 맞게 올바르게 업데이트되었습니다.모든 이벤트 타입이
TermsAgreementUiEvent로 일관되게 변경되었으며, 기존 기능이 유지되고 있습니다.
58-58: 내비게이션이 새로운 screens 모듈의 HomeScreen을 올바르게 사용하고 있습니다.중앙집중식 screen 정의를 통해 모듈 간 의존성이 올바르게 관리되고 있습니다.
63-67: UI 상태 반환 로직이 새로운 아키텍처에 맞게 올바르게 구현되었습니다.
TermsAgreementUiState를 사용하여 상태와 이벤트 처리를 깔끔하게 분리했습니다.
| data class SearchUiState( | ||
| val eventSink: (SearchUiEvent) -> Unit, | ||
| ) : CircuitUiState |
There was a problem hiding this comment.
🛠️ Refactor suggestion
UI 상태 구조 개선 제안
현재 SearchUiState는 eventSink만 포함하고 있어 실제 검색 상태 정보가 없습니다. 일반적으로 검색 기능에는 다음과 같은 상태가 필요합니다:
data class SearchUiState(
+ val query: String = "",
+ val searchResults: List<SearchResult> = emptyList(),
+ val isLoading: Boolean = false,
+ val error: String? = null,
val eventSink: (SearchUiEvent) -> Unit,
) : CircuitUiState또한 SearchUiEvent에도 구체적인 이벤트 정의가 필요합니다:
-sealed interface SearchUiEvent : CircuitUiEvent
+sealed interface SearchUiEvent : CircuitUiEvent {
+ data class QueryChanged(val query: String) : SearchUiEvent
+ data object Search : SearchUiEvent
+ data object ClearQuery : SearchUiEvent
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| data class SearchUiState( | |
| val eventSink: (SearchUiEvent) -> Unit, | |
| ) : CircuitUiState | |
| data class SearchUiState( | |
| val query: String = "", | |
| val searchResults: List<SearchResult> = emptyList(), | |
| val isLoading: Boolean = false, | |
| val error: String? = null, | |
| val eventSink: (SearchUiEvent) -> Unit, | |
| ) : CircuitUiState | |
| sealed interface SearchUiEvent : CircuitUiEvent { | |
| data class QueryChanged(val query: String) : SearchUiEvent | |
| data object Search : SearchUiEvent | |
| data object ClearQuery : SearchUiEvent | |
| } |
🤖 Prompt for AI Agents
In
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchUiState.kt
around lines 6 to 8, the SearchUiState data class currently only includes
eventSink and lacks actual search state information. To fix this, add properties
representing the search state such as the current query string, loading status,
and search results. Also, ensure that SearchUiEvent is expanded with specific
event definitions to handle user interactions properly.
| data class HomeUiState( | ||
| val eventSink: (HomeUiEvent) -> Unit, | ||
| ) : CircuitUiState | ||
|
|
||
| sealed interface HomeUiEvent : CircuitUiEvent |
There was a problem hiding this comment.
🛠️ Refactor suggestion
UI 상태 구조 개선 및 일관성 검토 필요
HomeUiState도 SearchUiState와 동일한 문제를 가지고 있습니다. 실제 홈 화면에 필요한 상태 정보가 없고, 이벤트 정의도 비어있습니다.
홈 기능에 필요한 구체적인 상태를 정의해주세요:
data class HomeUiState(
+ val isLoading: Boolean = false,
+ val featuredBooks: List<Book> = emptyList(),
+ val recentBooks: List<Book> = emptyList(),
+ val error: String? = null,
val eventSink: (HomeUiEvent) -> Unit,
) : CircuitUiState
-sealed interface HomeUiEvent : CircuitUiEvent
+sealed interface HomeUiEvent : CircuitUiEvent {
+ data object Refresh : HomeUiEvent
+ data class BookClicked(val bookId: String) : HomeUiEvent
+ data object NavigateToSearch : HomeUiEvent
+}패턴 일관성 유지 권장사항
모든 feature 모듈에서 동일한 UiState/UiEvent 패턴을 사용하고 있으므로, 각 기능별로 적절한 상태와 이벤트를 정의하여 일관성을 유지해주세요.
다른 feature 모듈들(library, login, termsagreement)에도 동일한 패턴을 적용하는 데 도움이 필요하시면 알려주세요.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| data class HomeUiState( | |
| val eventSink: (HomeUiEvent) -> Unit, | |
| ) : CircuitUiState | |
| sealed interface HomeUiEvent : CircuitUiEvent | |
| data class HomeUiState( | |
| val isLoading: Boolean = false, | |
| val featuredBooks: List<Book> = emptyList(), | |
| val recentBooks: List<Book> = emptyList(), | |
| val error: String? = null, | |
| val eventSink: (HomeUiEvent) -> Unit, | |
| ) : CircuitUiState | |
| sealed interface HomeUiEvent : CircuitUiEvent { | |
| data object Refresh : HomeUiEvent | |
| data class BookClicked(val bookId: String) : HomeUiEvent | |
| data object NavigateToSearch : HomeUiEvent | |
| } |
🤖 Prompt for AI Agents
In feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUiState.kt
around lines 6 to 10, the HomeUiState data class lacks any actual state
properties and the HomeUiEvent sealed interface is empty, which does not reflect
the real UI state or events needed for the home screen. To fix this, define
concrete state properties in HomeUiState that represent the home screen's UI
data (e.g., loading status, list of items, error messages) and add relevant
event subclasses to HomeUiEvent to handle user interactions or UI events. This
will align the home feature with the consistent UiState/UiEvent pattern used in
other feature modules.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
build.gradle.kts (1)
44-47: 외부 스크립트 적용 시 루트 기준 경로를 명시해 두는 것이 안전합니다
상대 경로"gradle/projectDependencyGraph.gradle"는settings.gradle.kts의rootDir가 변경되거나 하위 모듈에서 import 될 때 의도치 않게 깨질 수 있습니다.
rootProject.file()을 사용해 루트 기준으로 고정해 두는 편이 안전합니다.-apply { - from("gradle/projectDependencyGraph.gradle") -} +apply { + from(rootProject.file("gradle/projectDependencyGraph.gradle")) +}gradle/ProjectDependencyGraph.gradle (3)
54-71: API 의존성도 점선으로 표시하도록 보강 제안
현재endsWith('implementation')만 점선(edge style dotted) 처리합니다.api구성도 동일하게 처리하면 그래프 해석이 더 일관됩니다.-if (config.name.toLowerCase().endsWith('implementation')) { +if (config.name.toLowerCase().endsWith('implementation') || + config.name.toLowerCase().endsWith('api')) { traits.add('style=dotted') }
122-127:dot실행 실패시 친절한 메시지와 예외 처리 필요
dot명령이 없을 때도RuntimeException으로 빌드가 중단됩니다.
사용자에게 Graphviz 설치 방법을 안내하거나, 실행 가능 여부를 먼저 검사해 스킵하도록 하면 빌드 경험이 개선됩니다.
1-3: 산출물 파일명 충돌 가능성
고정 파일명project.dot/project.dot.png는 병렬 빌드나 멀티 체크아웃(예: 여러 브랜치를 동시에 빌드) 시 충돌할 수 있습니다. 타임스탬프나 해시를 포함해 유니크하게 만드는 것을 고려해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
build.gradle.kts(1 hunks)gradle/ProjectDependencyGraph.gradle(1 hunks)screens/build.gradle.kts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- screens/build.gradle.kts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#35
File: feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/TermsAgreementScreen.kt:127-127
Timestamp: 2025-07-09T01:14:29.836Z
Learning: In the Reed-Android project's TermsAgreementScreen.kt, the OnTermDetailClick event is intentionally passed an empty string for the URL parameter because the actual URLs for terms detail pages haven't been decided yet. This is a temporary implementation that will be updated once the URLs are finalized.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (1)
build.gradle.kts (1)
44-47: Graphviz 설치 여부에 대한 안내 필요
해당 스크립트에서dot커맨드를 호출하므로 Graphviz 가 설치되지 않은 빌드 머신에서는projectDependencyGraph작업이 실패합니다. CI·CD 환경에도 Graphviz 패키지를 포함했는지 확인해 주세요.
gradle/ProjectDependencyGraph.gradle
Outdated
| if (project.plugins.hasPlugin('com.android.dynamic-core')) { | ||
| androidDynamicFeatureProjects.add(project) | ||
| } |
There was a problem hiding this comment.
동적 피처 플러그인 ID 오타
Android Dynamic-Feature 모듈 플러그인 ID 는 com.android.dynamic-feature 입니다.
com.android.dynamic-core 는 존재하지 않아 동적 피처 모듈이 잘못 분류됩니다.
-if (project.plugins.hasPlugin('com.android.dynamic-core')) {
+if (project.plugins.hasPlugin('com.android.dynamic-feature')) {
androidDynamicFeatureProjects.add(project)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (project.plugins.hasPlugin('com.android.dynamic-core')) { | |
| androidDynamicFeatureProjects.add(project) | |
| } | |
| if (project.plugins.hasPlugin('com.android.dynamic-feature')) { | |
| androidDynamicFeatureProjects.add(project) | |
| } |
🤖 Prompt for AI Agents
In gradle/ProjectDependencyGraph.gradle around lines 47 to 49, the plugin ID for
Android Dynamic-Feature modules is incorrectly written as
'com.android.dynamic-core'. Replace 'com.android.dynamic-core' with the correct
plugin ID 'com.android.dynamic-feature' to ensure proper classification of
dynamic feature modules.
|
이 방법이 어떻게 가능한것인지에 대해선 추후 블로그글로 자세한 설명 작성 해보도록 하겠슴니다 |
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
gradle/projectDependencyGraph.gradle(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#35
File: feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/TermsAgreementScreen.kt:127-127
Timestamp: 2025-07-09T01:14:29.836Z
Learning: In the Reed-Android project's TermsAgreementScreen.kt, the OnTermDetailClick event is intentionally passed an empty string for the URL parameter because the actual URLs for terms detail pages haven't been decided yet. This is a temporary implementation that will be updated once the URLs are finalized.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (1)
gradle/projectDependencyGraph.gradle (1)
67-69: implementation 라인 스타일 지정 조건이 대·소문자 불일치 시 누락됩니다.
toLowerCase()이후endsWith('implementation')는 OK지만,apiImplementation,kaptImplementation등 의도치 않은 매칭 가능성이 있습니다. 정확히.*[Rr]eleaseImplementation$|.*[Dd]ebugImplementation$등으로 한정하거나 configuration 속성(Configuration.isCanBeResolved)을 분석하는 방법이 더 안전합니다.
| def p = "dot -Tpng -O ${dotFileName}".execute([], dot.parentFile) | ||
| p.waitFor() | ||
| if (p.exitValue() != 0) { | ||
| throw new RuntimeException(p.errorStream.text) | ||
| } | ||
| dot.delete() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
외부 프로세스 실행은 Gradle Exec API를 사용해 OS 종속성·오류 처리 개선이 필요합니다.
"dot -Tpng -O …".execute()는
- Graphviz 미설치 시 난해한 스택트레이스를 유발
- Windows 환경에서 실행 파일 확장자 문제 발생
- 실패 시
.dot파일이 삭제되지 않아 잔존 파일이 생김
Gradle exec {} 블록과 try/finally 를 사용해 예외-안전성과 호환성을 높이세요.
try {
exec {
commandLine 'dot', '-Tpng', dotFileName
workingDir dot.parentFile
}
} finally {
dot.delete() // 실패해도 항상 정리
}🤖 Prompt for AI Agents
In gradle/projectDependencyGraph.gradle around lines 122 to 127, replace the use
of "dot -Tpng -O ...".execute() with a Gradle exec { } block to run the external
process. Wrap the exec call in a try/finally block to ensure the .dot file is
deleted even if the command fails. Use commandLine with separate arguments
('dot', '-Tpng', dotFileName) and set workingDir to dot.parentFile. This
improves OS compatibility, error handling, and prevents leftover files.
| project.configurations.configureEach { config -> | ||
| if (config.name.toLowerCase().contains("test")) return | ||
| config.dependencies | ||
| .withType(ProjectDependency) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
테스트 관련 configuration 필터링 로직이 과도하게 넓습니다.
contains("test") 조건은 contest, latest, 등 예상치 못한 configuration 을 제외할 수 있습니다. 접두사 일치로 좁히거나 정규식을 사용하세요.
- if (config.name.toLowerCase().contains("test")) return
+ if (config.name.toLowerCase().startsWith("test")) return📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| project.configurations.configureEach { config -> | |
| if (config.name.toLowerCase().contains("test")) return | |
| config.dependencies | |
| .withType(ProjectDependency) | |
| project.configurations.configureEach { config -> | |
| if (config.name.toLowerCase().startsWith("test")) return | |
| config.dependencies | |
| .withType(ProjectDependency) |
🤖 Prompt for AI Agents
In gradle/projectDependencyGraph.gradle around lines 54 to 57, the filtering
logic for test-related configurations uses
config.name.toLowerCase().contains("test"), which is too broad and may exclude
unintended configurations like "contest" or "latest". Modify the condition to
check if the configuration name starts with "test" using a prefix match or apply
a regular expression to precisely match test-related configurations only.
|
feature 모듈간 의존성 해결 속이 다 시원하네요 고생하셨습니다 👍 |
|
아 feature:screens 로 둘걸그랬나.. |
아 저도 다음주 스터디 주제로 하려고 했는데 ㅋㅋㅋㅋㅋㅋㅋㅋㅋ 일단 저도 공부해보겠습니다 |
명분만 있으면 다 된다 아임니까~ |
🔗 관련 이슈
📙 작업 설명
🧪 테스트 내역 (선택)
📸 스크린샷 또는 시연 영상 (선택)
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
신규 기능
리팩터링
기타