-
Notifications
You must be signed in to change notification settings - Fork 0
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] 챌린지 함께 기록(친구) 페이지네이션 조회 API #246
Conversation
테스트통과 🤖 |
1 similar comment
테스트통과 🤖 |
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.
왜 다시올렸어요 ㅎㅎ 커밋기록땜시?
public List<TogetherChallengerResponse> getTogetherChallengers(Long challengeId) { | ||
public List<TogetherChallengerResponse> paginateTogetherChallengers(Long challengeId, Pageable pageable) { |
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.
TogetherChallenger라는 표현을 화면에서 표현하듯이 그냥 Friends로 표현해보는건어때요?
부사가 마치 형용사처럼 쓰이는게 좀 어색하다고 느껴지는것같아요.
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.
Friends가 더 적절해보이네요
.offset(pageable.getOffset()) | ||
.limit(pageable.getPageSize()) |
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.
왜 offset기반 페이징하시는건가요~??
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.
정렬 기준때문에 커서 페이징으로 하면 너무 복잡해져서요
네 다른 이슈에서 브랜치를 따버려서 ㅠ |
아! 그리고 단순 의견인데, 이런 수정은 리팩토링이라고 말하기는 힘든 것 같아요. |
ㅈㅅ |
테스트통과 🤖 |
관련 Issue
작업 내용
챌린지 함께 기록 API
테스트
PageableHandlerMethodArgumentResolver
추가Figma 링크