[Refactor] 로그제거#263
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앱 전반(~40개 파일)에서 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
No actionable comments were generated in the recent review. 🎉 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/konkuk/medicarecall/data/api/fcm/FcmService.kt (1)
131-133:⚠️ Potential issue | 🟡 Minor파이프라인 실패: 닫는 중괄호 앞 빈 줄 제거 필요
detekt에서
NoBlankLineBeforeRbrace위반이 보고되고 있어요. Line 132의 빈 줄을 제거하면 CI가 통과됩니다.🔧 수정 제안
} } - }
🧹 Nitpick comments (6)
app/src/main/java/com/konkuk/medicarecall/ui/feature/home/viewmodel/HomeViewModel.kt (2)
62-65:TAG상수가 더 이상 사용되지 않습니다.
Log.e/Log.d호출이 모두 제거되면서TAG상수가 dead code가 되었습니다. 함께 제거해 주세요.♻️ 수정 제안
private companion object { - const val TAG = "HomeViewModel" const val KEY_SELECTED_ELDER_ID = "selectedElderId" }
157-159: 사용되지 않는error파라미터를_로 변경하는 것을 권장합니다.
Log.e제거 후error가 더 이상 참조되지 않으므로, Kotlin 컨벤션에 맞게_로 바꾸면 깔끔합니다.♻️ 수정 제안
- .onFailure { error -> + .onFailure { _ ->app/src/main/java/com/konkuk/medicarecall/ui/feature/login/myinfo/viewmodel/LoginInfoViewModel.kt (1)
88-89: 빈 콜백 블록 — 에러 처리 부재 확인 필요로그 제거 후
onSuccess와onFailure가 모두 빈 블록으로 남아 있습니다. 인증번호 요청 실패 시 사용자에게 아무런 피드백이 없는 상태인데, 이전에도 로그만 찍고 있었으니 이 PR의 범위는 아니지만, 향후 실패 시 사용자에게 토스트나 이벤트를 emit하는 처리를 추가하면 좋을 것 같습니다.app/src/main/java/com/konkuk/medicarecall/domain/usecase/CheckLoginStatusUseCase.kt (2)
38-38: 빈.onSuccess { }블록 제거 가능로그 제거 후
.onSuccess { }가 no-op으로 남아있어요. 없어도 동작에 영향 없으니 깔끔하게 제거해도 좋을 것 같습니다.♻️ 제안
eldersInfoRepository.getCareCallTimes(it.key) - .onSuccess { } .onFailure { exception ->
42-43:errorBody변수가 사용되지 않고 남아있습니다로그 출력에서만 사용되던
errorBody가 제거 후 dead code로 남아있네요. 린트 경고 대상이 될 수 있습니다.♻️ 제안
is HttpException -> { val code = exception.code() - val errorBody = exception.response()?.errorBody()?.toString() when (code) {app/src/main/java/com/konkuk/medicarecall/data/repositoryimpl/FcmRepositoryImpl.kt (1)
55-63: 빈onSuccess/onFailure체인 제거 가능로그 제거 후
response.onSuccess { }.onFailure { }가 아무 일도 하지 않습니다.response변수 할당과 함께 정리하면 코드가 더 깔끔해집니다.♻️ 제안
// 서버에 갱신 요청 - val response: Result<Unit> = runCatching { + runCatching { fcmUpdateService.updateFcmToken( header = "Bearer $jwtToken", body = mapOf("fcmToken" to newToken), ) } - response.onSuccess { - }.onFailure { - }
| (fcmRepository as? FcmRepositoryImpl)?.validateAndRefreshTokenIfNeeded(jwtToken) | ||
| } catch (e: Exception) { | ||
| Log.e(TAG, "새 FCM 토큰 저장 중 오류 발생", e) | ||
| } catch (ignored: Exception) { |
There was a problem hiding this comment.
ingored 보다는 형식에 맞춰 e 로 유지하면 좋을 것 같아요
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/konkuk/medicarecall/ui/feature/login/calltime/component/FirstTimeWheelPicker.kt`:
- Around line 94-95: In FirstTimeWheelPicker.kt inside the FirstTimeWheelPicker
class there are six empty catch blocks causing Detekt EmptyCatchBlock failures;
for each empty catch (the ones already noted in your review) add the same
explanatory comment inside the block (for example: // 리플렉션 실패 시 무시) so the catch
blocks are no longer empty—update all six occurrences accordingly.
In
`@app/src/main/java/com/konkuk/medicarecall/ui/feature/login/calltime/component/ThirdWheelPicker.kt`:
- Around line 88-89: In ThirdWheelPicker
(app/src/main/java/com/konkuk/medicarecall/ui/feature/login/calltime/component/ThirdWheelPicker.kt)
replace the six empty catch block exception parameters currently named "e" with
the Kotlin unused-parameter underscore "_" (i.e., change catch (e: Exception) to
catch (_: Exception)) for the occurrences at the six empty catch blocks so
Detekt's EmptyCatchBlock exemption resumes and CI passes.
🧹 Nitpick comments (4)
app/src/main/java/com/konkuk/medicarecall/App.kt (1)
87-91: 빈 catch 블록 — 향후 멀티플랫폼 로거 도입 시 복원 고려
Log.d제거 취지는 이해하지만, 예외를 완전히 무시하면 FCM 토큰 저장 실패 시 프로덕션에서 원인 파악이 어려울 수 있어요. 브랜치명에 Napier/Kermit이 언급되어 있으니, 멀티플랫폼 로거 도입 시 이 부분에 로깅을 다시 추가하는 걸 추천드립니다. 🙂♻️ 향후 로거 도입 시 예시
appScope.launch { try { fcmRepository.saveFcmToken(token) } catch (e: Exception) { - // FCM 토큰 저장 실패 무시 + // TODO: 멀티플랫폼 로거(Napier/Kermit) 도입 후 복원 + // logger.e("FCM 토큰 저장 실패", e) } }app/src/main/java/com/konkuk/medicarecall/ui/feature/login/calltime/component/FirstTimeWheelPicker.kt (2)
78-96:hideDividers()함수가 6번 복붙되어 있어요.동일한 리플렉션 로직이
factory와update블록마다 반복됩니다.NumberPicker확장 함수로 한 번만 정의하면 파일 하단의setPickerTextColor/setPickerTextStyle과 같은 패턴으로 깔끔하게 정리할 수 있습니다.♻️ 확장 함수 추출 제안
파일 하단에 확장 함수를 추가하고:
`@SuppressLint`("DiscouragedPrivateApi", "SoonBlockedPrivateApi", "UseKtx") fun NumberPicker.hideDividers() { try { val dividerField = NumberPicker::class.java .getDeclaredField("mSelectionDivider") .apply { isAccessible = true } dividerField.set(this, Color.TRANSPARENT.toDrawable()) val heightField = NumberPicker::class.java .getDeclaredField("mSelectionDividerHeight") .apply { isAccessible = true } heightField.setInt(this, 0) } catch (e: Exception) { // 리플렉션 실패 시 무시 } }그리고 기존 6곳의 인라인
hideDividers()정의를 모두hideDividers()호출로 교체하면 됩니다.Also applies to: 109-127, 160-178, 191-209, 251-269, 282-300
63-76: 주석 처리된 코드가 상당량 남아 있어요.이 PR이 로그 제거 + 린트 정리 목적인데, 주석 처리된 코드 블록도 함께 정리하면 좋을 것 같아요. 나중에 필요하면 git history에서 복구할 수 있으니까요. 지금 당장 안 해도 괜찮지만 참고해주세요~ 🧹
Also applies to: 99-104, 129-136, 149-149, 152-158, 180-185, 211-218, 240-240, 243-249, 271-276, 302-309
app/src/main/java/com/konkuk/medicarecall/ui/feature/login/calltime/component/ThirdWheelPicker.kt (1)
72-90:hideDividers()가 6회 중복 정의되어 있어요동일한 리플렉션 로직이 factory/update 콜백마다 반복됩니다.
NumberPicker의 확장 함수로 한 번만 정의하면 유지보수가 훨씬 편해집니다. 또한 파일 전반에 주석 처리된 코드가 상당량 남아 있는데, 필요 없다면 정리하는 것도 좋겠습니다.`@SuppressLint`("DiscouragedPrivateApi", "SoonBlockedPrivateApi") private fun NumberPicker.hideDividers() { try { val dividerField = NumberPicker::class.java .getDeclaredField("mSelectionDivider") .apply { isAccessible = true } dividerField.set(this, Color.TRANSPARENT.toDrawable()) val heightField = NumberPicker::class.java .getDeclaredField("mSelectionDividerHeight") .apply { isAccessible = true } heightField.setInt(this, 0) } catch (_: Exception) { } }Also applies to: 103-121, 154-172, 185-203, 245-263, 276-294
| } catch (e: Exception) { | ||
| } |
There was a problem hiding this comment.
Detekt EmptyCatchBlock 에러로 CI 파이프라인이 실패하고 있어요.
Lines 350, 382의 catch 블록에는 // 리플렉션 실패 시 무시 주석이 있어서 Detekt를 통과하지만, 이 6곳은 catch 블록이 완전히 비어 있어서 에러가 발생합니다. 동일하게 주석을 추가해주세요.
🔧 수정 제안
각 빈 catch 블록에 주석을 추가하면 됩니다. 예시 (Line 94-95):
- } catch (e: Exception) {
- }
+ } catch (e: Exception) {
+ // 리플렉션 실패 시 무시
+ }나머지 5곳(Lines 125, 176, 207, 267, 298)도 동일하게 적용해주세요.
Also applies to: 125-126, 176-177, 207-208, 267-268, 298-299
🧰 Tools
🪛 GitHub Actions: Android CI
[error] 94-94: Detekt: EmptyCatchBlock detected. Empty catch block detected. If the exception can be safely ignored, name the exception according to the exemptions as per configuration.
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/konkuk/medicarecall/ui/feature/login/calltime/component/FirstTimeWheelPicker.kt`
around lines 94 - 95, In FirstTimeWheelPicker.kt inside the FirstTimeWheelPicker
class there are six empty catch blocks causing Detekt EmptyCatchBlock failures;
for each empty catch (the ones already noted in your review) add the same
explanatory comment inside the block (for example: // 리플렉션 실패 시 무시) so the catch
blocks are no longer empty—update all six occurrences accordingly.
| } catch (e: Exception) { | ||
| } |
There was a problem hiding this comment.
ignored → e 변경으로 Detekt EmptyCatchBlock 파이프라인 실패 발생
Detekt의 EmptyCatchBlock 규칙은 빈 catch 블록에서 예외 변수명이 설정된 면제 패턴(보통 ignored, expected 등)과 일치하면 허용합니다. 기존 ignored에서 e로 변경하면서 면제 조건이 깨져 CI가 실패하고 있습니다 (6곳 모두 동일).
수정 방법 (택 1):
- 변수명을
ignored로 되돌리기 - Kotlin 컨벤션에 맞게
_로 변경하기 (사용하지 않는 변수 표시)
🐛 제안: 모든 빈 catch 블록에서 변수명을 `_`로 변경
- } catch (e: Exception) {
+ } catch (_: Exception) {
}위 변경을 6곳 (Line 88, 119, 170, 201, 261, 292) 모두에 적용해 주세요.
Also applies to: 119-120, 170-171, 201-202, 261-262, 292-293
🧰 Tools
🪛 GitHub Actions: Android CI
[error] 88-88: Detekt: EmptyCatchBlock detected. Empty catch block detected. If the exception can be safely ignored, name the exception according to the exemptions as per configuration.
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/konkuk/medicarecall/ui/feature/login/calltime/component/ThirdWheelPicker.kt`
around lines 88 - 89, In ThirdWheelPicker
(app/src/main/java/com/konkuk/medicarecall/ui/feature/login/calltime/component/ThirdWheelPicker.kt)
replace the six empty catch block exception parameters currently named "e" with
the Kotlin unused-parameter underscore "_" (i.e., change catch (e: Exception) to
catch (_: Exception)) for the occurrences at the six empty catch blocks so
Detekt's EmptyCatchBlock exemption resumes and CI passes.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
detekt-config.yml (1)
73-83:⚠️ Potential issue | 🟠 Major
EmptyCatchBlock과SwallowedException를 전역적으로 비활성화하는 것은 위험할 수 있어요.Log.d 제거 후 빈 catch 블록이 생긴 건 이해하지만, 이 규칙들을 프로젝트 전체에서 꺼버리면 앞으로 발생하는 모든 빈 catch 블록과 삼켜지는 예외가 감지되지 않습니다. 이건 잠재적인 버그를 숨길 수 있는 부분이에요.
브랜치 이름(
refactor/Log.d-Naiper-Kermit-#262)에 Napier/Kermit이 언급되어 있는데, Log.d를 단순 삭제하기보다 멀티플랫폼 로거로 교체하는 게 원래 의도 아니었나요? 그렇게 하면 catch 블록이 비어있지 않게 되어 이 규칙들을 끌 필요도 없어질 거예요.당장 로거 교체가 어렵다면, 최소한 아래 대안을 고려해 주세요:
- 개별 catch 블록에
@Suppress("EmptyCatchBlock")어노테이션 적용 (전역 비활성화 대신)- 빈 catch 블록에 의도적으로 무시하는 이유를 주석으로 남기기
- 향후 로거 교체 작업을 추적할 이슈 생성
alswlekk
left a comment
alswlekk
left a comment
There was a problem hiding this comment.
ThiredWheelPicker 부분 파일만 한 번 더 확인 부탁드립니다 !
| .apply { isAccessible = true } | ||
| heightField.setInt(this, 0) | ||
| } catch (ignored: Exception) { | ||
| } catch (e: Exception) { |
There was a problem hiding this comment.
혹시 이파일에서 전체적으로 Exception을 ignored에서 e로 바꾼 이유가 있을까요 ??
There was a problem hiding this comment.
넵 ignored->e로 변경하면 좋을것같다고 해서 다른 파일들도 e로 수정했습니다!
Chore: #262 로그 제거
3 participants
🔗 관련 이슈
📙 작업 설명
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit