-
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 | CAKK-59 | External 모듈 리팩토링 및 Send 관련 추상화 #198
Conversation
Test Results 39 files 39 suites 26s ⏱️ Results for commit eb139ac. ♻️ This comment has been updated with latest results. |
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## develop #198 +/- ##
=============================================
+ Coverage 89.11% 89.17% +0.05%
+ Complexity 326 325 -1
=============================================
Files 110 109 -1
Lines 983 979 -4
Branches 37 36 -1
=============================================
- Hits 876 873 -3
- Misses 88 89 +1
+ Partials 19 17 -2
... and 6 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
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.
잘 변경해주신 것 같아요! 근데 인증 요청에 대한 템플릿을 하나로 합치는 것도 좋은것 같아요
Template의 기원이 대표적으로 Spring의 RestTemplate이 있는데요
RestTemplate의 GET, POST, PUT .. 등등 다양한 구현체들이 존재하듯이
메일이든 슬랙이든 모두 인증이라는 도메인으로 사용한다면 CertificationTemplate 내에 하나로 관리해도 좋을 것 같습니다
Extractor 관련 추상화 수준을 높여서 다시 push 해보겠습니다 ! |
@YongsHub PR이 커질거 같아서 해당 PR 머지 후 새로운 PR에 올리겠습니다 ! |
CAKK-59
ps. 기존에 활용하시던 CertificationApiExecutor의 역할이 MessageSender의 역할과 동일하여 교체하였습니다. 관련하여 의견 주시면, 의견에 따라 원복이나 현 상황 유지 및 Executor 삭제 하겠습니다.