Conversation
…ix-Android into feat/#101-notification-screen
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
| val context = LocalContext.current | ||
| val currentContext by rememberUpdatedState(context) | ||
|
|
||
| ObserveAsEvents(viewModel.sideEffect) { effect -> |
There was a problem hiding this comment.
이 부분을 이제 봤네 😓
나는 지금 람다 인자 이름을 sideEffect로 쓰고 있고 현수는 effect로 쓰고있는데 컨벤션을 통일하면 좋을 것 같아 ! 나는 굳이 줄이지 않아도 괜찮다고 생각하는데 현수 생각이 궁금하네
급한건 아니라 컨벤션 정하고 나중에 리팩터링할 때 반영하면 될듯 !
There was a problem hiding this comment.
지금 ImageVector로 리소스 렌더링하는 것도 그렇고 저희가 좀 통일이 안되는 부분이 있어서 한꺼번에 정리하는 게 나을 거 같아요
| (currentState.notificationList + it.notifications).distinctBy { n -> | ||
| n.id |
There was a problem hiding this comment.
n이라는 인자명이 어떤걸 의미하는지 파악하기 어려운 것 같아 ㅎㅎ
| android { | ||
| namespace = "com.twix.notification" | ||
| } |
There was a problem hiding this comment.
내가 멀티 모듈에 대한 이해가 아직 부족해서 그런데 이 namespace가 core:notification의 namespace와 동일한데 문제되는 부분이 없을까 ?
There was a problem hiding this comment.
R이나 BuildConfig 같은 거를 두 모듈에서 안 쓰고 있어서 당장은 문제가 없긴 한데 그래도 유니크하게 바꾸는 게 좋겠네요
…ix-Android into feat/#101-notification-screen
2 participants
이슈 번호
작업내용
결과물
2026-02-26.12.09.30.mov
리뷰어에게 추가로 요구하는 사항 (선택)
가로 모드 제어하는 설정 추가했습니다!