-
Notifications
You must be signed in to change notification settings - Fork 70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[feature/#300] Apply Type Safe Navigation #301
Conversation
Test Results18 tests 18 ✅ 5s ⏱️ Results for commit ee817b8. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋습니다 👍
(참고) 이러한 변경사항을 안전하게 적용하기 위해 MainScreenTest
이 있는데용,
이 PR 변경사항 때문은 아니지만 지금은 깨지고 있네요 😂
interface Route | ||
|
||
interface MainTabRoute : Route |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sealed interface를 활용하여 컴파일러에게 구현된 타입을 알려주면 어떨까요?
Route, MainTabRoute별로 subclass를 묶어두면 가독성도 좋아질 것 같아요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드리뷰 적용하셨습니다
|
[feature/droidknights#300] Apply code review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Issue
Overview (Required)
시연 영상
Screen_recording_20240530_115044.mp4