Skip to content
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/modelmapper migration #263

Merged
merged 16 commits into from
Dec 27, 2021
Merged

Conversation

jyeonjyan
Copy link
Member

한 일

  • modelmapper -> querydsl: 실제개선사례 전체 조회
  • improvementService: exception 터지는 조건 if 조건으로 처리. (명확히 명시)
  • testcode: 기존의 하드코딩 -> junit5 메서드를 골고루 활용 함.

개선이 필요해 보이는 부분

  • Admin 비즈니스 로직 return type 정규화
  • TestCode 스타일 정규화 (테스트코드에서 비즈니스 로직을 사용해 변경에 대해 비용이 생김)

참고

Screen Shot 2021-12-25 at 11 43 51 PM

Exception Handling 부분에 대한 검증 누락.
Admin 로직 개편 | Test 작성 규칙 의논 후 검증 예정.

리뷰

  • 메서드 명명규칙
  • 빠뜨린 주석, 예외 등등

@jyeonjyan jyeonjyan added build success This branch was built successfully decision making You need to make a decision clean I'm clearing my legacy labels Dec 25, 2021
Copy link
Member

@siwony siwony left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다. 코멘트 확인해주세요

Copy link
Contributor

@dolong2 dolong2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

확인했습니다

@jyeonjyan jyeonjyan merged commit 5ac1274 into develop Dec 27, 2021
@delete-merged-branch delete-merged-branch bot deleted the feature/modelmapper-migration branch December 27, 2021 00:21
@jyeonjyan jyeonjyan mentioned this pull request Jan 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build success This branch was built successfully clean I'm clearing my legacy decision making You need to make a decision
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants