-
Notifications
You must be signed in to change notification settings - Fork 1
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
[REFACTOR] response body 구조 개선 및 단건 조회 로직 구현부 변경 #101
Conversation
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.
질문 사항 남겨두었습니다 !
@@ -38,11 +37,11 @@ public ApiResponseDto<AuthLoginResponseDto> socialLogin(@RequestBody AuthRequest | |||
|
|||
// 로그인 | |||
if (!responseDto.isNewUser()) { | |||
return ApiResponseDto.success(SuccessStatus.SIGNIN_SUCCESS, responseDto); |
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.
그리고 여기 제가 리팩토링 하면서 발견한 부분이긴 한데 ,,,!
if절과 else절로 나눈 이유가 내려주는 data의 형식은 같은데, 단순히 SuccessStatus 값의 차이 때문이었던걸로 판단했는데, 맞을까요 ?? @Seokyeong237
그 이유 때문이라면 지금 저희 response body는 message가 들어가지 않기 때문에 if-else 문으로 나눌 필요 없을 것 같은데 어떻게 생각하실까요 ?
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.
@yeseul106 헉 요거 놓쳤네요 :( 맞아요 이제 나눌 필요 없습니다 수정해주셔서 감사해용 ㅎㅎ
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.
수고하셨습니다~ 리뷰 확인 부탁드려요 !~
.status(status.getHttpStatus().value()) | ||
.success(true) | ||
.message(status.getMessage()) | ||
.status("SSS") |
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.
p3;
클래스 변수로 선언해주시는 것이 좋을 것 조금 더 좋을 것 같습니다!
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.
확인이요 !!
📝 Summary
👩💻 Contents
📝 Review Note
📣 Related Issue
📬 Reference