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

[10] 상품 등록 #17

Merged
merged 10 commits into from
Sep 1, 2024
Merged

[10] 상품 등록 #17

merged 10 commits into from
Sep 1, 2024

Conversation

ohsuha
Copy link
Collaborator

@ohsuha ohsuha commented Aug 29, 2024

No description provided.

@ohsuha
[10-product-CRUD] DDL sql 추가
2fba194
@ohsuha
[10-prduct-CRUD] 상품 등록
f1eb85d 
- 상품 등록을 위한 카테고리 조회
- 상품 등록
@ohsuha ohsuha requested a review from mason136 August 29, 2024 15:41
@ohsuha ohsuha self-assigned this Aug 29, 2024
@ohsuha ohsuha linked an issue Aug 29, 2024 that may be closed by this pull request
파트너 회원 : 상품 등록 #10
Closed
@mason136
Copy link

develop을 기본 브랜치로 바꿔주시고, feature 브랜치가 develop에 머지가 안됐으면, feature 브랜치에서 새로운 feature 브랜치를 따서 작업해주세요~~
PR은 피쳐프랜치로 머지하게 만들어주시면 됩니다.
지금 같은 경우 feature/10-product-CRUD -> 6-create-user-and-partner 이렇게 PR 올려주시면 되요

@mason136
Copy link

공통적인 API 응답 포맷과 에러 응답 객체를 만들어주세요
에러 코드별로 특정 응답 상태랑 메세지, 코드를 내려주면 좋겠습니다.

@ohsuha
Copy link
Collaborator Author

ohsuha commented Aug 30, 2024 •
edited
Loading

6-create-user-and-partner 를 머지해버려서.....
혹시 이번만.. main 에 바로 머지하고

담부터는

develop(구 main)
       ㄴ commerce_basic (feature들은 여기서 브랜치 땀)
          ㄴ feature/[2]_create_user
          ㄴ feature/ [10]_create_product
          ㄴ ....
      ㄴ security
          ㄴ feature/[22]_security_filter
          ㄴ ...

이렇게 가라는 말씀이신가요?????

@mason136
Copy link

네 맞습니다

mason136
mason136 reviewed Aug 31, 2024
View reviewed changes
src/main/java/org/example/commerce_site/common/response/CommonResponse.java Outdated

public static <T> CommonResponse.CommonData<T> success(T data) {
return (CommonResponse.CommonData<T>) CommonData.builder()
.message(MESSAGE_SUCCESS)
Copy link

Choose a reason for hiding this comment

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

성공시 message는 안내보내도 되긴 해요

src/main/java/org/example/commerce_site/common/response/CommonResponse.java Outdated
@Builder
@NoArgsConstructor
@AllArgsConstructor
public static class CommonData<T> {
Copy link

Choose a reason for hiding this comment

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

성공 응답인 경우만 CommonData 쓰는 걸로 보이는데 code랑 message는 필요 없어요

Copy link

@mason136 mason136 Aug 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

이렇게 중첩클래스로 정의하면 API response return 타입 명시할 때 마다 CommonResponse.CommonData 이렇게 써야하니, 길어지는 단점이 있는데
중첩 클래스를 안쓰는게 더 편하지 않을까요??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ApiFailResponse, ApiSuccessResponse 로 분리했습니다

src/main/java/org/example/commerce_site/common/response/CommonResponse.java Outdated
@Builder
@AllArgsConstructor
@NoArgsConstructor
public class CommonResponse {
Copy link

Choose a reason for hiding this comment

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

Common이란 네이밍보단 ApiResponse 이렇게 명시하는게 더 정확하지 않을까요??

@ohsuha
[10] failResponse , success response 를 분리
8175f94 
- 성공시 불필요한 code, message 제거
@ohsuha ohsuha merged commit bcb70a4 into develop Sep 1, 2024
1 check passed
@ohsuha ohsuha deleted the feature/10-product-CRUD branch September 3, 2024 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mason136 mason136 mason136 approved these changes

Assignees

@ohsuha ohsuha

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

파트너 회원 : 상품 등록
2 participants
@ohsuha @mason136