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.
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
[MOD] 보관함 - 내가 그린 코스 / 사용자가 설정한 제목이 표시 되도록 수정 #310
[MOD] 보관함 - 내가 그린 코스 / 사용자가 설정한 제목이 표시 되도록 수정 #310
Changes from 8 commits
11095da
62cdb81
f7d481d
e1f5a75
012cf62
be2a8d8
e8132c5
e832657
6685b7b
ac4d6e8
059b8f2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
기존 코드이긴 하지만 BaseResponse가 생겼으니 Response 바꾸는 김에 적용하면 더욱 간결할 것 같아요 😄
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.
@sxunea 감사합니다. 수정하였습니다~!
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.
null 처리 기본 값 이대로 지정해도 괜찮을 지 확인 부탁드립니다.
기존엔 하나라도 null이면 runCatching에 잡혀서 onFailure로 빠지고 있더라구요
일단은 임시로 null 처리 해두었습니다.
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.
기본값을 세팅한다는 건 앱을 죽이지 않고 살리겠다는 것인데 포인트는 DrawActivity의 data가 CountDown -> Run 액티비티로 넘어간다는 것입니다. 앱은 안 죽었지만 사용자가 의도했던 것과 다른 코스/출발지 등등으로 세팅되어 RunActivity가 실행되는 것은 의미가 없다고 판단하여
현재 아래와 같이 값에 이상이 있을 경우 토스트 메세지를 띄우고 페이지를 안 넘기게 막아놨습니다.
이와 관련된 논의 스레드입니다.
https://w1670909332-6ay129099.slack.com/archives/C04EXNB86LB/p1703075472448109
null 처리 기본값 지정 시 위의 toast 메세지 로직이 안 먹히게 되어 이 부분은 기본값 없이 두는 게 좋을 것 같습니다.
혹시 더 좋은 의견이 있다면 궁금합니다 :)
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.
그러면 RunActivity와 같이 올바른 기능 수행이 어려울 때는 페이지를 넘기지 않고, GET과 같은 데이터를 가져와서 ui에 붙이는 통신들(ex. 마라톤 코스 GET)은 기본값을 지정해주는 것으로 이해하면 될까요 ? 앱이 터지지 않는 것도, 의미 있는 사용도 챙기다보니 기준이 좀 모호한 것 같아서요 다른 분들도 생각이 궁금합니다 !
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.
@sxunea
기준이라는 개념보다는 그때그때 상황 보고 기획 단과 논의하여 결정하면 될 것 같습니다! 경우의 수가 다양하니까요
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.
@unam98 해당 부분 Revert 해두었습니다~!