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.

Sign up for GitHub

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

Jump to bottom

MemberCategoryTemplateApplicationService 테스트 #655

Merged
merged 2 commits into from
Sep 20, 2024
Merged

MemberCategoryTemplateApplicationService 테스트 #655

merged 2 commits into from
Sep 20, 2024

Conversation

kyum-q
Copy link
Contributor

@kyum-q kyum-q commented Sep 16, 2024 •
edited
Loading

⚡️ 관련 이슈

close #645

📍주요 변경 사항

MemberCategoryTemplateApplicationService Test 구현

🎸기타

#646 에 CI 테스트 문제에 대해 이야기 해보고 싶음
일단 해당 PR은 CI 문제가 발생하지 않도록 세팅해놓음

@kyum-q kyum-q added refactor 요구사항이 바뀌지 않은 변경사항 BE 백엔드 labels Sep 16, 2024
@kyum-q kyum-q added this to the 5차 스프린트🍗 milestone Sep 16, 2024
@kyum-q kyum-q self-assigned this Sep 16, 2024
@kyum-q kyum-q changed the base branch from main to dev/be September 16, 2024 16:27
zangsu
zangsu reviewed Sep 18, 2024
View reviewed changes
Copy link
Contributor

@zangsu zangsu left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 켬미!
이번 리뷰도 이전과 마찬가지로 정말 사소한 의견들을 남겨 보았어요.
한번 읽어보시고 마음에 드는 쪽으로 결정해 주세요~

.../test/java/codezap/category/service/facade/MemberCategoryTemplateApplicationServiceTest.java Outdated Show resolved Hide resolved
.../test/java/codezap/category/service/facade/MemberCategoryTemplateApplicationServiceTest.java Outdated Show resolved Hide resolved
.../test/java/codezap/category/service/facade/MemberCategoryTemplateApplicationServiceTest.java Outdated Show resolved Hide resolved
.../test/java/codezap/category/service/facade/MemberCategoryTemplateApplicationServiceTest.java Outdated Show resolved Hide resolved
.../test/java/codezap/category/service/facade/MemberCategoryTemplateApplicationServiceTest.java Outdated Show resolved Hide resolved
.../test/java/codezap/category/service/facade/MemberCategoryTemplateApplicationServiceTest.java Outdated Show resolved Hide resolved
.../test/java/codezap/category/service/facade/MemberCategoryTemplateApplicationServiceTest.java Outdated Show resolved Hide resolved
jminkkk
jminkkk previously approved these changes Sep 19, 2024
View reviewed changes
Copy link
Contributor

@jminkkk jminkkk left a comment

Choose a reason for hiding this comment

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

안녕하세요 켬미
수고하셨습니다 🔥
충분히 잘 작성된 것 같아 approve하겠습니다 🍀

.../test/java/codezap/category/service/facade/MemberCategoryTemplateApplicationServiceTest.java Outdated Show resolved Hide resolved
.../test/java/codezap/category/service/facade/MemberCategoryTemplateApplicationServiceTest.java Outdated Show resolved Hide resolved
zeus6768
zeus6768 requested changes Sep 19, 2024
View reviewed changes
Copy link
Contributor

@zeus6768 zeus6768 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

고생 많았어요 켬미 👍👍👍👍

남긴 코멘트에서 의견 나누고 싶어 RC 남깁니다!

그런데 질문이 있어요.

본 PR에서는 새로운 테스트를 생성할 뿐인데 Refactor 라벨이 적절한지 의문입니다!

CI 테스트 문제에 대해 이야기 해보고 싶음

해당 CI 테스트 문제에 대해 조금만 더 자세히 언급해줄 수 있을까요~?

.../test/java/codezap/category/service/facade/MemberCategoryTemplateApplicationServiceTest.java Outdated Show resolved Hide resolved
.../test/java/codezap/category/service/facade/MemberCategoryTemplateApplicationServiceTest.java Outdated Show resolved Hide resolved
@kyum-q
Copy link
Contributor Author

kyum-q commented Sep 19, 2024 •
edited
Loading

@zeus6768

해당 CI 테스트 문제에 대해 조금만 더 자세히 언급해줄 수 있을까요~?

허걱 제가 앞에 링크를 잘못 걸어뒀었네요. !

#646 <- 이 PR 기타에 CI문제에 자세하게 적어놨습니다 !

본 PR에서는 새로운 테스트를 생성할 뿐인데 Refactor 라벨이 적절한지 의문입니다!

test 코드 구현이 기능 추가가 아니니 feat이 아니라고 판단해 refactor를 달았습니다. 그런데 이것도 제우스 말처럼 적절하지 않다고 생각이 들기도 하네요.. refactor 라벨도 제거하도록 하겠습니다~

@kyum-q kyum-q removed the refactor 요구사항이 바뀌지 않은 변경사항 label Sep 19, 2024
jminkkk
jminkkk approved these changes Sep 20, 2024
View reviewed changes
Copy link
Contributor

@jminkkk jminkkk left a comment

Choose a reason for hiding this comment

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

켬미 짱

HoeSeong123
HoeSeong123 approved these changes Sep 20, 2024
View reviewed changes
Copy link
Contributor

@HoeSeong123 HoeSeong123 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

zeus6768
zeus6768 approved these changes Sep 20, 2024
View reviewed changes
Copy link
Contributor

@zeus6768 zeus6768 left a comment

Choose a reason for hiding this comment

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

고생많았어요 켬미~

#646 에서 언급한 CI 문제는 글만 봐서는 의도를 이해하기 어려운 것 같아요.

이슈나 디스커션으로 분리해서 정리하면 더 좋겠어요!

zangsu
zangsu approved these changes Sep 20, 2024
View reviewed changes
Copy link
Contributor

@zangsu zangsu left a comment

Choose a reason for hiding this comment

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

켬미 너무 수고 많으셨어요~~~~ 고켬~~~

@zangsu zangsu merged commit 2d0af11 into dev/be Sep 20, 2024
5 checks passed
@zangsu zangsu deleted the test/645-member-category-template-application-service branch September 20, 2024 10:29
@zangsu zangsu mentioned this pull request Sep 27, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zangsu zangsu zangsu approved these changes

@zeus6768 zeus6768 zeus6768 approved these changes

@jminkkk jminkkk jminkkk approved these changes

@HoeSeong123 HoeSeong123 HoeSeong123 approved these changes

Assignees

@kyum-q kyum-q

Labels
BE 백엔드
Projects
2024-code-zap
Status: Weekend Done
Milestone
5차 스프린트🍗
Development

Successfully merging this pull request may close these issues.

[REFACTOR] 서비스 테스트
5 participants
@kyum-q @zangsu @zeus6768 @jminkkk @HoeSeong123