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

기본 이미지 삭제하지 않는 기능 추가 #727

Merged
merged 2 commits into from
Oct 19, 2023
Merged

기본 이미지 삭제하지 않는 기능 추가 #727

merged 2 commits into from
Oct 19, 2023

Conversation

jjongwa
Copy link
Member

@jjongwa jjongwa commented Oct 19, 2023

📄 Summary

name이 기본 이미지인지 판단하는 로직이 이벤트 리스너에 있는게 맞을까요?

🙋🏻 More

@jjongwa jjongwa self-assigned this Oct 19, 2023
@github-actions
Copy link

github-actions bot commented Oct 19, 2023 •
edited
Loading

📝 Jacoco Test Coverage

Total Project Coverage 78.99% ❌
File Coverage [81.82%]
PublishEventListener.java 100% 🍏
S3ImageEventListener.java 58.33% ❌

hgo641
hgo641 approved these changes Oct 19, 2023
View reviewed changes
Copy link
Collaborator

@hgo641 hgo641 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.

전........괜찮다고 생각 . . .
아니면 ImageService에서 deleteImage() 메서드를 만들어서 이벤트를 발행하게 하고, 다른 코드에서는 직접 이벤트를 발행하는 대신 ImageService의 deleteImage()를 사용하게해도 좋겠네요 근데 이러면 TripSerivce가 ImageService에 의존하게 돼서 테스트 코드등 수정사항이 좀 있겠군요. 지금 하기에 시간이 부족하면 데모데이 이후 ㅎ 에 개선하는 것도 나쁘지 않을지도

mcodnjs
mcodnjs approved these changes Oct 19, 2023
View reviewed changes
Copy link
Collaborator

@mcodnjs mcodnjs left a comment

Choose a reason for hiding this comment

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

default-image.png 상수로 빼도 .. 히히 ..

@jjongwa jjongwa merged commit 563a140 into develop Oct 19, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@waterricecake waterricecake waterricecake approved these changes

@hgo641 hgo641 hgo641 approved these changes

@mcodnjs mcodnjs mcodnjs approved these changes

@LJW25 LJW25 LJW25 approved these changes

Assignees

@jjongwa jjongwa

Labels
BE 🔨 Refactor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jjongwa @LJW25 @mcodnjs @hgo641 @waterricecake