-
Notifications
You must be signed in to change notification settings - Fork 70
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
[Week8] Make API Docs, Docker image #2
[Week8] Make API Docs, Docker image #2
Conversation
상품 서비스가 어떤 역할을 하는지 설명하기 위해 주석을 작성하였습니다.
상품 컨트롤러가 어떤 역할을 하는지 설명하기 위해 주석을 작성하였습니다.
세션 컨트롤러가 어떤 역할을 하는지 설명하기 위해 주석을 작성하였습니다.
회원 컨트롤러, 서비스가 어떤 역할을 하는지 설명하기 위해 주석을 작성하였습니다.
app/src/main/java/com/codesoom/assignment/application/UserService.java
Outdated
Show resolved
Hide resolved
app/src/main/java/com/codesoom/assignment/application/ProductService.java
Outdated
Show resolved
Hide resolved
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.
app/src/main/java/com/codesoom/assignment/application/ProductService.java
Outdated
Show resolved
Hide resolved
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.
그 사이에 주석 작성 실력이 일취월장하셨군요.
app/src/main/java/com/codesoom/assignment/controllers/ProductController.java
Outdated
Show resolved
Hide resolved
클래스의 구현 내용이 주석에 포함되어 있었습니다. 구현 내용은 언제든 변할 수 있어 주석에 작성하는 것이 올바르지 않다고 생각합니다. 그래서 클래스의 핵심 역할을 제외한 설명들을 지웠습니다.
인증 서비스의 역할을 설명하였습니다.
상품의 역할을 설명하였습니다.
회원의 역할을 설명하였습니다.
반복되는 단어로 주석을 한번에 이해하기 어려웠습니다. 그래서 반복된 단어를 의미가 비슷한 다른 단어로 변경하였습니다.
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.
669d9f6 좋습니다. 그러나 마지막 줄은 없어도 괜찮았을 거에요.
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.
e57fa1e 이런 경우엔 커밋 메시지 타이틀을 다음과 같이 적고 본문은 적지 않아도 괜찮을 것 같습니다.
app/src/main/java/com/codesoom/assignment/controllers/UserController.java
Show resolved
Hide resolved
반복되는 단어로 주석을 한번에 이해하기 어려웠습니다. 그래서 반복된 단어를 의미가 비슷한 다른 단어로 변경하였습니다.
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.
app/src/main/java/com/codesoom/assignment/errors/ProductNotFoundException.java
Outdated
Show resolved
Hide resolved
app/src/main/java/com/codesoom/assignment/errors/UserEmailDuplicationException.java
Outdated
Show resolved
Hide resolved
app/src/main/java/com/codesoom/assignment/errors/UserNotFoundException.java
Outdated
Show resolved
Hide resolved
REST Api 문서작성을 위해 Spring REST Docs와 관련된 의존성을 추가하였습니다.
현재 프로젝트에 설정된 자바8 버전에서는 공백을 확인하는 메소드(isBlank)를 사용할 수 없습니다. 따라서, 자바 버전을 8 -> 11로 변경하였습니다.
한줄에 모든 속성이 표현되어 한번에 파악하기가 어려웠습니다. prettyPrint 설정을 추가하여 한줄에 하나의 속성을 표현하도록 변경하였습니다.
app/src/docs/asciidoc/index.adoc
Outdated
====== http | ||
|
||
include::{path}/http-request.adoc[] |
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.
위에 이미 http request에 대한 정보가 제공되니 이 부분은 지워도 될 것 같네요.
.andExpect(content().string(containsString("쥐돌이"))) | ||
.andDo(document("get-products", | ||
responseFields( | ||
fieldWithPath("[].id").type(NUMBER).description("상품 식별자"), | ||
fieldWithPath("[].name").type(STRING).description("상품 이름"), | ||
fieldWithPath("[].maker").type(STRING).description("상품 제조사"), | ||
fieldWithPath("[].price").type(NUMBER).description("상품 가격"), | ||
fieldWithPath("[].imageUrl").type(STRING).optional().description("상품 이미지") | ||
)) | ||
) | ||
; |
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.
REST docs를 위한 코드들은 따로 분리해도 좋을 것 같네요.
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.
네. 배열이나 리스트로 다른 곳에 선언하는 방법을 사용해 보세요.
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.
45384ae 내용은 사실이지만 필요 이상으로 자세한 설명입니다. 주석을 올바른 형식으로 고쳐라
정도가 적당해요.
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.
c3ea2c6 개발과 관련된 문서의 종류는 다양합니다. 어떤 문서인지 명시해주면 바람직할 거에요.
다음과 같이 수정하면 어떨까요?
API 문서 작성을 위해 asciidoctor 의존성을 추가하라
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.
나쁘지 않은 주석이지만 isBlank
를 위해 11을 선택한 것 같이 좀 미묘한 느낌이 듭니다. isBlank
를 해결하기 위해서라면 11이 아니라 13도 15도 가능했겠죠.
회원 요청 정보 클래스들에 기본생성자가 없어 데이터 바인딩에 문제가 있었습니다.
테스트 코드 안에 문서작성을 위한 코드가 같이 있어 클래스가 복잡했습니다. 그래서, 두 코드를 분리하였습니다. 문서작성을 위한 코드에 중복된 값들이 많았습니다. 중복된 값들을 없애기 위해 Item, ItemProvider 클래스를 생성하여 요청, 응답, 헤더, 파라미터 값들을 관리하였습니다.
공감합니다.. 8, 11, 13, 15 차이를 잘 모르는데 학습을 해야할 것 같습니다. |
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.
요청 필드가 필수값인지 아닌지에 대한 정보를 사용자에게 제공하기 위해 사용자 지정 템플릿을 작성하였습니다.
Item.of("[].id", NUMBER, "상품 식별자"), | ||
Item.of("[].name", STRING, "상품 이름"), | ||
Item.of("[].maker", STRING, "상품 제조사"), | ||
Item.of("[].price", NUMBER, "상품 가격"), | ||
Item.of("[].imageUrl", STRING, "상품 이미지", State.OPTIONAL) |
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.
여기에서 [].
은 상위의 of("[]", of("id", NUMBER, "상품 식별자"), of("name", STRING, "상품 이름"), ... )
처럼 한 단계 더 분리할 수 있습니다.
} | ||
|
||
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.
fieldWithPath("email").type(STRING).description("회원 이메일"), | ||
fieldWithPath("name").type(STRING).description("회원 이름"), | ||
fieldWithPath("password").type(STRING).description("회원 비밀번호") | ||
new ItemProvider(EMAIL, NAME, PASSWORD).toFields() |
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.
좋은 시도입니다.
아직 이번 한주도 많이 배웠습니다. 너무 감사했습니다 ㅎ.ㅎ |
많이 배우셨다니 저도 기쁩니다. |
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.
주간 회고를 작성해 공유해주세요. 한 주간 고생 많으셨습니다.
* refactor: 패키지 구조를 hexagonal-architecture 구조로 변경 * refactor: HelloController 삭제
해야 할 일
벌써 8주차네요 ㅎ.ㅎ 이번 한주도 잘 부탁드립니다^_^