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

Dao- changing from then to async #491

Closed
wants to merge 2 commits into from
Closed

Dao- changing from then to async #491

wants to merge 2 commits into from

Conversation

hanyiseo2
Copy link
Contributor

Why:

What's being changed:

except for 4 files in Dao, changed codes which use then to async

@hanyiseo2
Copy link
Contributor Author

에러 잡아서 다시 커밋 할게요!

pagingDTO ? pagingDTO.sequelizeOption() : {}
)
);
// const result = await Ingredient.findAndCountAll(
Copy link
Contributor

Choose a reason for hiding this comment

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

불필요한 주석은 모두 지워주면 좋겠다는 생각이 들어요!

이전 코드를 봐야 할 필요가 있다면 git에 남아있는 이력을 추적하면 되니까요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵 알겠습니다!

);
});
return new ListAndCountDTO<BrandDTO>(result.count, rows);
} catch (error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

에러 catch 이후에 별다른 처리 없이 바로 throw를 한다면, try-catch 를 굳이 하지 않아도 좋을거 같아요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵! 수정하겠습니다!

@neverlish
Copy link
Contributor

PR의 base branch를 main이 아니라 dev 로 바꿔주세요~

@hanyiseo2 hanyiseo2 changed the base branch from main to dev May 19, 2023 23:17
@neverlish neverlish closed this Jun 13, 2023
@neverlish neverlish deleted the then-async branch June 13, 2023 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants