Open
Conversation
📝 WalkthroughWalkthroughMyPage 모듈의 여러 뷰(NotificationView, PersonalizationView, SettingsView, WeeklyReportView)에서 레이아웃 일관성을 개선했습니다. VStack의 spacing과 padding 값을 표준화(horizontal 32, vertical 16 등)하고, 상태 변경 감지를 위한 Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🎓 코치의 피드백잘한 점: 개선 제안:
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
e0ng
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
TAMINGO/Features/MyPage/Personalization/View/PersonalizationView.swift (2)
91-95:onChange클로저의newValue파라미터가 사용되지 않습니다
newValue를 선언했지만 클로저 본문에서 전혀 사용되지 않습니다. iOS 17+에서는 파라미터 없는 단축 형태를 사용할 수 있습니다.✨ 간결하게 정리
-.onChange(of: viewModel.personalizationSetting) { _, newValue in +.onChange(of: viewModel.personalizationSetting) { Task { await viewModel.updateSetting() } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TAMINGO/Features/MyPage/Personalization/View/PersonalizationView.swift` around lines 91 - 95, The onChange closure currently declares unused parameters ("_, newValue") when observing viewModel.personalizationSetting; remove the unused parameters and use the parameterless shorthand closure (or single implicit parameter) so the body simply starts the Task and calls await viewModel.updateSetting(); update the call site of onChange(of: viewModel.personalizationSetting) to omit the unused parameters and keep the Task { await viewModel.updateSetting() } body, referencing onChange, viewModel.personalizationSetting, and viewModel.updateSetting to locate the change.
87-90: 명시적인await로 병렬 작업을 명확하게 조정하세요
async let _패턴은 Swift에서 문법적으로 유효하지만(SE-0317), 의도가 불명확합니다. 이 패턴은 바인딩된 값을 읽지 않으므로 스코프 종료 시점에 자동으로 취소/대기되는데, 이는 개발자에게 예상치 못한 일시 중단점을 만들 수 있습니다.세 작업이 병렬로 실행되고 모두 완료될 때까지 대기한다는 의도를 명확히 하려면, 명시적으로 이름을 붙이고
await로 조정하세요. 이는 Swift 공식 커뮤니티 가이드라인의 권장 패턴입니다:♻️ 개선 제안
.task { - async let _ = viewModel.loadSetting() - async let _ = viewModel.loadStatistics() - async let _ = viewModel.loadRecent() + async let setting: Void = viewModel.loadSetting() + async let statistics: Void = viewModel.loadStatistics() + async let recent: Void = viewModel.loadRecent() + _ = await (setting, statistics, recent) }이렇게 작성하면 구조화된 동시성(structured concurrency)의 fork-join 패턴이 명확해지고, 의도하지 않은 스코프 종료 대기 동작을 피할 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TAMINGO/Features/MyPage/Personalization/View/PersonalizationView.swift` around lines 87 - 90, The three parallel calls using "async let _ = viewModel.loadSetting()", "async let _ = viewModel.loadStatistics()", and "async let _ = viewModel.loadRecent()" should be made explicit: declare named async lets (e.g., let setting = async let ... , let stats = async let ... , let recent = async let ...) and then await them (await setting, await stats, await recent) so the fork-join semantics are clear and you avoid implicit cancellation/await at scope exit; update the code that currently uses those unnamed async lets to use the named bindings and explicit awaits.TAMINGO/Features/MyPage/WeeklyReport/Model/InsightType.swift (1)
41-41:borderColor와titleColor가 둘 다.subPink2로 동일합니다다른 케이스들은 border와 title 색상이 서로 구분되어 있습니다:
.productivity:subGreen2/subGreen3.arrival:subYellow2/subYellow3.task:subBlue3/subBlue2
.suggestion만borderColor(Line 41)와titleColor(Line 71) 둘 다.subPink2를 사용합니다. 의도된 디자인 결정이라면 문제없지만, 일관성 차원에서 한 번 더 확인해보세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TAMINGO/Features/MyPage/WeeklyReport/Model/InsightType.swift` at line 41, In the InsightType enum's .suggestion case, borderColor and titleColor are both set to .subPink2; update the titleColor for .suggestion from .subPink2 to .subPink3 (i.e., change the titleColor value in the .suggestion branch) so the title and border follow the same contrasting pattern used by other cases (refer to borderColor, titleColor and the .suggestion case in InsightType).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@TAMINGO/Features/MyPage/Setting/SettingsView.swift`:
- Around line 29-34: The SettingCard instances in SettingsView.swift currently
use print(...) placeholders (e.g., the "업데이트 확인" and "오픈소스 라이선스" SettingCard
actions and others like "서비스 이용약관", "개인정보 처리방침", "위치 정보 이용약관", "의견 보내기") —
replace each print call with an appropriate real navigation/action: use
NavigationLink for in-app screens (e.g., license or settings detail), call
UIApplication.shared.open(_:) for external URLs (terms/privacy/location), or if
the implementation is pending add a clear TODO comment (e.g., // TODO: implement
navigation for "업데이트 확인") so no inert print remains in release builds; update
the closures attached to SettingCard to perform the chosen action and keep
identifiers like SettingCard and the title strings to locate each item.
- Around line 27-34: The SettingCard for "업데이트 확인" currently hardcodes sub: "최신
버전"; change it to a non-misleading placeholder (e.g., empty string or "확인 중")
until actual version-comparison logic is implemented. Locate the SettingCard
invocation (SettingCard(title: "업데이트 확인", sub: ...)) in SettingsView and replace
the literal "최신 버전" with a safe default or viewModel-provided state (e.g., use
viewModel.appVersion or an empty string/placeholder) so the UI doesn't claim the
app is up-to-date prematurely.
---
Nitpick comments:
In `@TAMINGO/Features/MyPage/Personalization/View/PersonalizationView.swift`:
- Around line 91-95: The onChange closure currently declares unused parameters
("_, newValue") when observing viewModel.personalizationSetting; remove the
unused parameters and use the parameterless shorthand closure (or single
implicit parameter) so the body simply starts the Task and calls await
viewModel.updateSetting(); update the call site of onChange(of:
viewModel.personalizationSetting) to omit the unused parameters and keep the
Task { await viewModel.updateSetting() } body, referencing onChange,
viewModel.personalizationSetting, and viewModel.updateSetting to locate the
change.
- Around line 87-90: The three parallel calls using "async let _ =
viewModel.loadSetting()", "async let _ = viewModel.loadStatistics()", and "async
let _ = viewModel.loadRecent()" should be made explicit: declare named async
lets (e.g., let setting = async let ... , let stats = async let ... , let recent
= async let ...) and then await them (await setting, await stats, await recent)
so the fork-join semantics are clear and you avoid implicit cancellation/await
at scope exit; update the code that currently uses those unnamed async lets to
use the named bindings and explicit awaits.
In `@TAMINGO/Features/MyPage/WeeklyReport/Model/InsightType.swift`:
- Line 41: In the InsightType enum's .suggestion case, borderColor and
titleColor are both set to .subPink2; update the titleColor for .suggestion from
.subPink2 to .subPink3 (i.e., change the titleColor value in the .suggestion
branch) so the title and border follow the same contrasting pattern used by
other cases (refer to borderColor, titleColor and the .suggestion case in
InsightType).
Comment on lines
+27
to
+34
| print("버전 정보 탭") | ||
| } | ||
| SettingCard(title: "업데이트 확인", sub: "최신 버전", subColor: .mainMint) { | ||
| print("업데이트 확인 탭") | ||
| } | ||
| SettingCard(title: "오픈소스 라이선스") { | ||
| print("오픈소스 라이선스 탭") | ||
| } |
There was a problem hiding this comment.
"업데이트 확인"의 sub 텍스트 "최신 버전"이 하드코딩되어 있습니다
현재는 실제 버전 비교 없이 항상 "최신 버전"을 표시합니다. 사용자에게 잘못된 정보를 줄 수 있는 UI입니다. viewModel.appVersion을 실제 앱스토어 최신 버전과 비교하는 로직이 연결되기 전까지는 빈 문자열이나 다른 상태 표시로 대체하는 것을 권장합니다.
🛡️ 임시 처리 제안
-SettingCard(title: "업데이트 확인", sub: "최신 버전", subColor: .mainMint) {
+SettingCard(title: "업데이트 확인", sub: viewModel.updateStatus) {
print("업데이트 확인 탭")
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TAMINGO/Features/MyPage/Setting/SettingsView.swift` around lines 27 - 34, The
SettingCard for "업데이트 확인" currently hardcodes sub: "최신 버전"; change it to a
non-misleading placeholder (e.g., empty string or "확인 중") until actual
version-comparison logic is implemented. Locate the SettingCard invocation
(SettingCard(title: "업데이트 확인", sub: ...)) in SettingsView and replace the
literal "최신 버전" with a safe default or viewModel-provided state (e.g., use
viewModel.appVersion or an empty string/placeholder) so the UI doesn't claim the
app is up-to-date prematurely.
Comment on lines
+29
to
+34
| SettingCard(title: "업데이트 확인", sub: "최신 버전", subColor: .mainMint) { | ||
| print("업데이트 확인 탭") | ||
| } | ||
| SettingCard(title: "오픈소스 라이선스") { | ||
| print("오픈소스 라이선스 탭") | ||
| } |
There was a problem hiding this comment.
신규 메뉴 항목들의 print 플레이스홀더 액션이 남아 있습니다
"업데이트 확인", "오픈소스 라이선스", "서비스 이용약관", "개인정보 처리방침", "위치 정보 이용약관", "의견 보내기" 모두 print(...) 만 실행합니다. 릴리즈 빌드에서도 동작 없는 버튼이 노출됩니다.
실제 네비게이션(NavigationLink, UIApplication.shared.open(_:) 등) 또는 TODO 주석으로 대체를 권장합니다.
추적 이슈로 등록하거나, 각 항목별 네비게이션 로직을 구현하는 초안이 필요하시면 도움드릴 수 있습니다.
Also applies to: 51-68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@TAMINGO/Features/MyPage/Setting/SettingsView.swift` around lines 29 - 34, The
SettingCard instances in SettingsView.swift currently use print(...)
placeholders (e.g., the "업데이트 확인" and "오픈소스 라이선스" SettingCard actions and others
like "서비스 이용약관", "개인정보 처리방침", "위치 정보 이용약관", "의견 보내기") — replace each print call
with an appropriate real navigation/action: use NavigationLink for in-app
screens (e.g., license or settings detail), call UIApplication.shared.open(_:)
for external URLs (terms/privacy/location), or if the implementation is pending
add a clear TODO comment (e.g., // TODO: implement navigation for "업데이트 확인") so
no inert print remains in release builds; update the closures attached to
SettingCard to perform the chosen action and keep identifiers like SettingCard
and the title strings to locate each item.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✨ PR 유형
어떤 변경 사항이 있나요??
🛠️ 작업내용
[ 작업한 내용을 작성해주세요 ( UI 구현이라면 사진도 같이 올려주시면 좋아요! ) ]
📋 추후 진행 상황
[ 다음에 진행할 작업에 대해 작성해주세요!! ]
[ 현재 커밋 후 풀리퀘 다음으로 작업 내용을 적어주면 됩니다! ]
📌 리뷰 포인트
[ 어떤 부분을 잘 체크해야하는지 작성해주세요 ]
✅ Checklist
PR이 다음 요구 사항을 충족하는지 확인해주세요!!!
Summary by CodeRabbit
릴리스 노트
새로운 기능
스타일