-
Notifications
You must be signed in to change notification settings - Fork 2
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
Feat/#498 Artist 엔티티 추가, 가수 검색 기능 구현 #502
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.
바론... 혼자 무슨 싸움을 해오신 겁니까...
2300 줄 보고 깜짝 놀랐네요 진짜
너무너무너무너무너무 고생 많았습니다 !! 중간 중간에 고쳐야 할 부분이 있는 것 같아 RC 남겼어요. 내일 같이 이야기해보죠!
수고 많았습니다 진짜로.... 바론은 신이다
backend/src/main/java/shook/shook/song/application/ArtistSearchService.java
Show resolved
Hide resolved
backend/src/main/java/shook/shook/song/application/ArtistSearchService.java
Show resolved
Hide resolved
backend/src/main/java/shook/shook/song/application/ArtistSearchService.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/shook/shook/song/application/ArtistSearchService.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/shook/shook/song/application/ArtistSearchService.java
Outdated
Show resolved
Hide resolved
backend/src/test/java/shook/shook/song/domain/ArtistNameTest.java
Outdated
Show resolved
Hide resolved
backend/src/test/java/shook/shook/song/ui/ArtistSongSearchControllerTest.java
Outdated
Show resolved
Hide resolved
backend/src/test/java/shook/shook/song/ui/HighLikedSongControllerTest.java
Show resolved
Hide resolved
backend/src/test/java/shook/shook/voting_song/application/VotingSongServiceTest.java
Outdated
Show resolved
Hide resolved
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.
많이 반영되어서 좋아졌군요 !
바론의 뛰어난 설명 덕에 모두 납득했습니다 ^^
다만 아직 조금 궁금한 부분이 있긴 한데, 내일 이야기해보시죵 어프루브할게요!
// id 로 song 찾아온느 쿼리 1개 -> 비즈니스 로직에서 조건 필터링 => 2번을 1번으로 | ||
// 100개 이하의 데이터는 비즈니스 로직에서 정렬하는 것을 추천한다. | ||
// 조건이 확실하게 있는 경우는 쿼리에서 하는 것이 좋다. |
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.
이 부분은 반영된 부분인가요?!
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.
충격적인 사실 전해드립니다.. 저희 쿼리를 보면서 테코톡을 준비하다가 반영해보면 좋겠다! 라는 생각이 들어 작성해 둔 주석을 지우는걸 까먹었습니다 😂
📝작업 내용
💬리뷰 참고사항
DB 변경사항 운영서버: 반드시 코드가 배포되기 전에 DB를 먼저 변경해야 함
도와주세요 🆘
현재 DB에 있는 가수명과 동의어를 새롭게 추가해야 합니다.
그런데 스쿼드 인원이 줄어서 데이터를 수집하기 어려운 상황.. 여러분의 도움이 필요합니다. (간절)
#️⃣연관된 이슈
close #498