[Refactor] profile 외부 모듈로 분리 및 파일 오류 수정 - #1531
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough프로필 기능을 독립적인 Changes프로필 모듈 분리
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@feature/profile/src/main/java/in/koreatech/koin/feature/profile/component/ProfileTimetableDefaults.kt`:
- Around line 6-11: Move the hardcoded Korean weekday values from
ProfileTimetableDefaults.days into a profile_timetable_days string array
resource, then remove that defaults property and load the localized array with
stringArrayResource in the ProfileTimetable composable. Preserve the existing
weekday ordering and usage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 850f77de-98ad-45c9-ac56-be753385249a
📒 Files selected for processing (26)
feature/home/src/main/java/in/koreatech/koin/feature/home/profile/Constant.ktfeature/profile/build.gradle.ktsfeature/profile/consumer-rules.profeature/profile/lint-baseline.xmlfeature/profile/proguard-rules.profeature/profile/src/main/AndroidManifest.xmlfeature/profile/src/main/java/in/koreatech/koin/feature/profile/Constant.ktfeature/profile/src/main/java/in/koreatech/koin/feature/profile/ProfileScreen.ktfeature/profile/src/main/java/in/koreatech/koin/feature/profile/ProfileSideEffect.ktfeature/profile/src/main/java/in/koreatech/koin/feature/profile/ProfileState.ktfeature/profile/src/main/java/in/koreatech/koin/feature/profile/ProfileViewModel.ktfeature/profile/src/main/java/in/koreatech/koin/feature/profile/component/ProfileCard.ktfeature/profile/src/main/java/in/koreatech/koin/feature/profile/component/ProfileTimetable.ktfeature/profile/src/main/java/in/koreatech/koin/feature/profile/component/ProfileTimetableDefaults.ktfeature/profile/src/main/java/in/koreatech/koin/feature/profile/mapper/ProfileTimetableMapper.ktfeature/profile/src/main/java/in/koreatech/koin/feature/profile/model/ProfileTimetableLecture.ktfeature/profile/src/main/res/drawable/ic_profile_avatar.xmlfeature/profile/src/main/res/drawable/ic_profile_login.xmlfeature/profile/src/main/res/drawable/ic_profile_logout.xmlfeature/profile/src/main/res/drawable/ic_profile_notification.xmlfeature/profile/src/main/res/drawable/ic_profile_notification_dot.xmlfeature/profile/src/main/res/drawable/ic_profile_setting.xmlfeature/profile/src/main/res/values/string.xmlkoin/build.gradle.ktskoin/src/main/java/in/koreatech/koin/ui/newmain/fragment/ProfileFragment.ktsettings.gradle
💤 Files with no reviewable changes (1)
- feature/home/src/main/java/in/koreatech/koin/feature/home/profile/Constant.kt
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
||
| val timeCellPadding = remember { PaddingValues(horizontal = 8.dp, vertical = 6.dp) } | ||
| val headerCellPadding = remember { PaddingValues(horizontal = 8.dp, vertical = 6.dp) } | ||
| val cellPadding = PaddingValues(horizontal = 8.dp, vertical = 6.dp) |
There was a problem hiding this comment.
[Minor] PaddingValues 객체가 리컴포지션마다 새로 생성됩니다.
PaddingValues(...) 는 Composable 스코프에서 remember 없이 선언되면 매 리컴포지션마다 새 객체가 할당됩니다. 구조적 동등성 덕분에 하위 컴포저블의 리컴포지션을 유발하지는 않지만, 불필요한 힙 할당이 발생합니다.
| val cellPadding = PaddingValues(horizontal = 8.dp, vertical = 6.dp) | |
| val cellPadding = remember { PaddingValues(horizontal = 8.dp, vertical = 6.dp) } |
|
|
||
| lectures.forEachIndexed { index, lecture -> | ||
| key(index) { | ||
| key("${lecture.dayOfWeek}_${lecture.startTotalMinutes}_${lecture.name}") { |
There was a problem hiding this comment.
[Minor] key() 에 문자열 보간 대신 다중 인자를 사용하세요.
"${a}_${b}_${c}" 형태의 문자열 보간은 리컴포지션마다 String 객체를 새로 할당합니다. Compose의 key() 는 여러 인자를 받아 CompoundKey 로 처리하므로, 문자열 할당 없이 동일한 안정성을 제공합니다.
또한 동명의 강의가 같은 요일·같은 시작 시간에 배치될 경우 키가 중복될 수 있습니다. 현실적으로 드문 케이스지만 Compose 런타임 경고의 원인이 됩니다.
| key("${lecture.dayOfWeek}_${lecture.startTotalMinutes}_${lecture.name}") { | |
| key(lecture.dayOfWeek, lecture.startTotalMinutes, lecture.name) { |
| import kotlin.collections.firstOrNull | ||
| import kotlin.collections.orEmpty |
There was a problem hiding this comment.
[Trivial] 불필요한 명시적 표준 라이브러리 임포트입니다.
kotlin.collections.firstOrNull 과 kotlin.collections.orEmpty 는 Kotlin 기본 스코프에 자동으로 포함되어 있으므로 명시적으로 임포트할 필요가 없습니다. IDE가 자동 생성한 것으로 보이며, 삭제해도 컴파일에 영향 없습니다.
| import kotlin.collections.firstOrNull | |
| import kotlin.collections.orEmpty |
|
|
||
| Image( | ||
| imageVector = ImageVector.vectorResource(R.drawable.ic_koin_text), | ||
| contentDescription = "" |
There was a problem hiding this comment.
[Minor] 빈 문자열 contentDescription은 접근성을 해칩니다.
contentDescription = ""으로 설정하면 TalkBack이 해당 요소를 읽으려 시도하지만 아무것도 말하지 않아 스크린 리더 사용자에게 혼란을 줍니다. 장식용 이미지(로고 등)는 null로 설정해야 Compose가 접근성 트리에서 제외시킵니다. 위의 ic_bcsd_symbol 이미지도 동일하게 적용해야 합니다.
| contentDescription = "" | |
| contentDescription = null |
| @Composable | ||
| private fun ProfileIcon( | ||
| imageVector: ImageVector, | ||
| modifier: Modifier = Modifier | ||
| ) { | ||
| Box( | ||
| modifier = modifier | ||
| .clip(RoundedCornerShape(10.dp)) | ||
| .background(RebrandKoinTheme.colors.neutral100) | ||
| .padding(vertical = 8.dp, horizontal = 8.dp), | ||
| contentAlignment = Alignment.Center | ||
| ) { | ||
| Icon( | ||
| modifier = Modifier.size(24.dp), | ||
| imageVector = imageVector, | ||
| contentDescription = null, | ||
| tint = RebrandKoinTheme.colors.primary500 | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
[Info] ProfileIcon은 HomeIcon의 로직을 중복합니다. core/designsystem으로 이동을 고려해주세요.
ProfileIcon은 feature/home의 HomeIcon과 거의 동일한 구현입니다. 현재 feature/profile이 feature/home에 의존할 수 없기 때문에 로컬 복제는 이해되지만, 이런 패턴이 다른 feature 모듈에도 반복될 경우 유지보수 비용이 증가합니다.
중장기적으로 HomeIcon (또는 더 범용적인 이름인 KoinIcon)을 core/designsystem으로 이동하면 모든 feature 모듈이 공유해서 사용할 수 있습니다. 이번 PR 스코프에서 즉시 해결할 필요는 없지만 별도 이슈로 추적을 권장합니다.
| android:pathData="M17.499,9C18.427,9 19.317,8.631 19.973,7.975C20.63,7.319 20.999,6.428 20.999,5.5C20.999,4.572 20.63,3.681 19.973,3.025C19.317,2.369 18.427,2 17.499,2C16.57,2 15.68,2.369 15.024,3.025C14.367,3.681 13.998,4.572 13.998,5.5C13.998,6.428 14.367,7.319 15.024,7.975C15.68,8.631 16.57,9 17.499,9Z" | ||
| android:strokeLineJoin="round" | ||
| android:strokeWidth="1.5" | ||
| android:fillColor="#00000000" |
There was a problem hiding this comment.
[Minor] 알림 점(dot)의 색상이 하드코딩되어 있습니다.
strokeColor="#980AC9" 는 테마 색상 토큰과 연결되지 않은 매직 컬러입니다. Vector drawable에서 런타임 틴팅을 사용하는 것이 어렵다면, 최소한 디자인 시스템의 컬러 팔레트에 해당 값이 정의되어 있는지 확인하고 컬러 이름을 주석으로 남겨 추적 가능하게 해주세요.
android:strokeColor="#980AC9"
| <string name="profile_setting">설정</string> | ||
| <string name="profile_timetable_section">내 시간표</string> | ||
| <string name="profile_username_default">익명</string> | ||
| </resources> No newline at end of file |
There was a problem hiding this comment.
[Trivial] 파일 끝에 개행 문자가 없습니다.
POSIX 표준 및 프로젝트 git 컨벤션에 따라 텍스트 파일의 마지막 줄에 개행 문자(\n)가 있어야 합니다. proguard-rules.pro와 AndroidManifest.xml도 동일하게 수정이 필요합니다.
| Column( | ||
| modifier = modifier | ||
| .fillMaxWidth() | ||
| .padding(top = 16.dp, start = 24.dp, end = 24.dp), | ||
| verticalArrangement = Arrangement.spacedBy(12.dp) |
There was a problem hiding this comment.
[Info] TimetableSection 리팩토링 — 잘 처리했습니다.
HomeSection 컴포넌트(다른 모듈 의존성)를 제거하고 Column + Arrangement.spacedBy로 직접 구현한 것은 올바른 방향입니다. 타이포그래피도 medium18 → bold18로 교체하여 디자인 스펙을 더 정확히 반영했습니다. 👍
bcsd-b-bot
Bot
added
ci:passed
and removed
ci:passed
coderabbit:complete
claude:complete
labels
KYM-P
left a comment
KYM-P
left a comment
There was a problem hiding this comment.
Padding 값 remember 과
문자열 단일 key 대신 다중 key 만 처리해주시면 될 거 같아요
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@feature/profile/src/main/java/in/koreatech/koin/feature/profile/component/ProfileTimetable.kt`:
- Line 151: Update the key call in the timetable composition to add spaces after
each comma, matching the ktlint formatting style while preserving its existing
arguments.
- Line 151: Update the key in the ProfileTimetableLecture rendering block to use
lecture.id as the stable unique identifier, or include another field that
guarantees uniqueness alongside the existing values. Preserve the surrounding
timetable rendering behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ce5e1bbc-9b7d-45fe-8101-2cb3095bf5cc
📒 Files selected for processing (1)
feature/profile/src/main/java/in/koreatech/koin/feature/profile/component/ProfileTimetable.kt
|
|
||
| val timeCellPadding = remember { PaddingValues(horizontal = 8.dp, vertical = 6.dp) } | ||
| val headerCellPadding = remember { PaddingValues(horizontal = 8.dp, vertical = 6.dp) } | ||
| val cellPadding = remember { PaddingValues(horizontal = 8.dp, vertical = 6.dp) } |
There was a problem hiding this comment.
[Info] ✅ remember 제거 — 올바른 결정
ProfileTimetableDefaults.START_HOUR / END_HOUR 는 const val, days 는 object 의 val (한 번만 초기화)이므로 remember 로 캐싱할 이유가 없습니다. 불필요한 remember 를 제거한 것이 정확합니다. cellPadding 은 PaddingValues 객체 할당이므로 remember 유지가 적절합니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@feature/profile/src/main/java/in/koreatech/koin/feature/profile/ProfileScreen.kt`:
- Around line 80-84: Update the onSettingClick callback in ProfileScreen so
non-logged-in users navigate to the login flow after logging the LOGIN_PROMPT
event, without calling onNavigateToSetting; invoke onNavigateToSetting only when
uiState.isLoggedIn is true.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e342ffda-02f2-46ed-aef7-5593294ae8b0
📒 Files selected for processing (4)
feature/profile/src/main/java/in/koreatech/koin/feature/profile/Constant.ktfeature/profile/src/main/java/in/koreatech/koin/feature/profile/ProfileScreen.ktfeature/profile/src/main/java/in/koreatech/koin/feature/profile/component/ProfileTimetable.ktkoin/src/main/java/in/koreatech/koin/ui/newmain/fragment/ProfileFragment.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- feature/profile/src/main/java/in/koreatech/koin/feature/profile/Constant.kt
- feature/profile/src/main/java/in/koreatech/koin/feature/profile/component/ProfileTimetable.kt
b37cf9c to
6834d7f
Compare
|
3 participants
PR 개요
이슈 번호: #1496
PR 체크리스트
작업사항
작업사항의 상세한 설명
profile을 home 내부에서 외부 모듈로 분리했습니다
파일에 있는 사용하지 않는 코드 몇가지를 제거했습니다
논의 사항
스크린샷
추가내용
Summary by CodeRabbit
새 기능
개선 사항