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

[Refact] Transfer reviewDao js to ts #360

Merged
merged 5 commits into from
Aug 30, 2022

Conversation

ChoHadam
Copy link
Member

기존 reviewDao.js를 제거하고 revireDao.ts를 작성해 새로 연결했습니다.

구현 후 mocha test passing하는것 확인했고,
local server로 돌렸을때 Review 관련 기능들 잘 동작하는지 Swagger docs로 확인했습니다.

@ChoHadam
Copy link
Member Author

@heesung6701
아직 ReviewDTO가 없어서 타입 정의를 모호하게 지정해놓은 점 리뷰시 참고 부탁드립니다 :)

@ChoHadam ChoHadam linked an issue Aug 30, 2022 that may be closed by this pull request
4 tasks
@heesung6701
Copy link
Member

@heesung6701
아직 ReviewDTO가 없어서 타입 정의를 모호하게 지정해놓은 점 리뷰시 참고 부탁드립니다 :)

한번에 수정하는것보다 이렇게 차근차근 수정하는게 리뷰하기도 좋습니다 :)

@heesung6701
Copy link
Member

LGTM

Copy link
Member

@heesung6701 heesung6701 left a comment

Choose a reason for hiding this comment

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

리뷰 남겼습니다:) 중괄호 만 추가해주면 될거 같네요.

src/dao/ReviewDao.ts Outdated Show resolved Hide resolved
src/dao/ReviewDao.ts Outdated Show resolved Hide resolved
src/dao/ReviewDao.ts Show resolved Hide resolved
nest: true,
});

if (readReviewResult === null) {
Copy link
Member

Choose a reason for hiding this comment

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

undefined 인 경우도 이후 코드를 수행하지 않는게 안전할 거 같습니다! undefined인 경우는 존재하지 않아야 하지만..

Copy link
Member Author

Choose a reason for hiding this comment

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

💭 undefined을 return할 수도 있다는 가능성을 염두하고 체킹할거라면, 이 경우 뿐 만 아니라 함수를 호출하고 return 값을 사용하는 모든 부분에서 undefined 체크를 해야한다고 생각합니다.
undefined를 전반적으로 허용하지않도록 처리하는 방법을 구글링해봤지만 아직 찾지 못했습니다..
찾게 된다면 공유하고 수정하겠습니다.

Copy link
Member

@heesung6701 heesung6701 left a comment

Choose a reason for hiding this comment

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

approved

@ChoHadam ChoHadam changed the title Refact/issue 359 tranfer review dao js to ts [Refact] Transfer reviewDao js to ts Aug 30, 2022
@ChoHadam ChoHadam merged commit ff766d7 into dev Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Refactoring] js -> ts 변환 작업
2 participants