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

[JT-47] S3 파일 업로더 구현 #1

Merged
merged 3 commits into from
Sep 1, 2023
Merged

[JT-47] S3 파일 업로더 구현 #1

merged 3 commits into from
Sep 1, 2023

Conversation

kmebin
Copy link
Member

@kmebin kmebin commented Sep 1, 2023

📌 개발 내용

  • s3Template 이용
  • application.yml s3 profile 분리
  • .gitignore에 application-*.yml, !application.yml 추가

📑 PR 포인트

  • 커스텀 Exception은 추후에 적용할 예정
  • UploadController는 테스트입니다..^^ 제송

👥 협업을 위한 코드리뷰

  1. 세상에 바보같은 질문은 없다.
  2. 실수를 예방하는 팀이 아니라, 실수를 잘 다루는 팀이 되자.
  3. 분업이 아닌 협업을 하자. 우린 모두 같은 문제를 이겨내기 위해 모였다.
  4. 영원한 것은 없으니 대화를 하자.
  5. 서로 존중하자.
  6. 많이 부딪치고 깨닫고 배우자.
  7. 부드럽게 설득하고 열린 마음으로 설득당해보자.

✅ 리마인더

  • 본인의 로컬에서 정상 동작하는지 확인해주세요.
  • 최신 브랜치를 Pull 받고 PR을 요청했는지 확인해주세요.
  • API가 추가되었을 경우 테스트를 하고 PR을 올려주세요.
  • Conflict가 났을 때, UI상에서 해결하지 말고, 본인 local에서 해결해주세요.
  • Commit 메시지 제대로 작성해주세요.

⚙️ 코드리뷰 룰

  • R(Request Change): 해당 블럭은 꼭 변경해주셨으면 좋겠습니다.
  • C(Comment): 웬만하면 고려해주시면 좋겠습니다.
  • Q(Question) : 해당 라인이 궁금합니다.
  • A(Approve): 반영해도 좋고 넘어가도 좋습니다. 혹은 사소한 의견입니다.

@kmebin
[JT-47] feat: S3 파일 업로더 구현
94d0ee9 
- application.yml s3 profile 분리
parksey
parksey reviewed Sep 1, 2023
View reviewed changes
Copy link
Collaborator

@parksey parksey left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

module-application/src/main/java/com/devtoon/jtoon/webtoon/UploadController.java Show resolved Hide resolved
module-domain-s3/src/main/java/com/devtoon/jtoon/S3Uploader.java Show resolved Hide resolved
hongdosan
hongdosan reviewed Sep 1, 2023
View reviewed changes
Copy link
Member

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

C: 희빈님, 재윤님 고생하셨습니다. 다만 커밋을 좀 더 세세하게 나누면 좋을 것 같아요!! 지금처럼 하나의 PR에 하나의 Commit이 좀 어색하기두 하고 정확하게 나누어져 있다면 더 이해가 잘 될 것 같아요!

module-domain-s3/src/main/java/com/devtoon/jtoon/S3Uploader.java Outdated
private final S3Template s3Template;

@Value("${cloud.aws.s3.bucket}")
private String bucket;
Copy link
Member

@hongdosan hongdosan Sep 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

C: 상수 부분은 대문자로 하는 게 규칙인데, @value로 값을 불러오는 필드도 상수라고 생각해서 대문자로 하는게 어떨까요? 제 생각입니다! ㅎㅎ

Copy link
Member Author

Choose a reason for hiding this comment

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

좋은디요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

굿인디요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

쪼은디요?

module-domain-s3/src/main/java/com/devtoon/jtoon/S3Uploader.java Outdated
private String bucket;

public void upload(String directory, MultipartFile file) {
String key = directory + "/" + UUID.randomUUID();
Copy link
Member

@hongdosan hongdosan Sep 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

C: "/" 부분은 매직넘버로 분리하면 좋을 것 같아요! 어떤가요?

Copy link
Member Author

Choose a reason for hiding this comment

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

수정했슴다 ㄱㅅㄱㅅ요

ymkim97
ymkim97 approved these changes Sep 1, 2023
View reviewed changes
Copy link
Collaborator

@ymkim97 ymkim97 left a comment

Choose a reason for hiding this comment

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

LGTM

@kmebin kmebin merged commit b6799f8 into develop Sep 1, 2023
1 check passed
@kmebin kmebin changed the title [JT-47] feat: S3 파일 업로더 구현 [JT-47] S3 파일 업로더 구현 Sep 1, 2023
@kmebin kmebin deleted the feature/JT-47 branch September 14, 2023 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hongdosan hongdosan hongdosan left review comments

@ymkim97 ymkim97 ymkim97 approved these changes

@parksey parksey parksey approved these changes

@Shin-Jae-Yoon Shin-Jae-Yoon Shin-Jae-Yoon approved these changes

Assignees

@kmebin kmebin

@Shin-Jae-Yoon Shin-Jae-Yoon

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kmebin @hongdosan @parksey @ymkim97 @Shin-Jae-Yoon