-
Notifications
You must be signed in to change notification settings - Fork 22
[김재욱] Sprint12 #66
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
[김재욱] Sprint12 #66
The head ref may contain hidden characters: "express-\uAE40\uC7AC\uC6B1"
Conversation
[박민규] sprint 6, 7
Express 김재욱 mission11
…n-be into express-김재욱-TS
Mission 12
basilry
left a comment
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.
전체적으로 DDD에 맞게 잘 짜셨습니다.
다만 폴더 구조는 어느정도 잡혀있는데, 파일의 내용이 조금 폴더 성격과 안맞게 구성되어 있는 부분들이 있었어요. 내용이 분절되어 있다던지같은.
이런 부분들은 차차 개선되어 나갈거라 생각합니다.
고생하셨습니다.
| }); | ||
|
|
||
| if (!writerEntity) { | ||
| throw new NotFoundException(ExceptionMessage.USER_NOT_FOUND); |
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.
오, 에러를 따로 별도 모듈화를 하신 부분은 잘하셨습니다.
| import { NotFoundException } from "../../exceptions/NotFoundException"; | ||
| import { ForbiddenException } from "../../exceptions/ForbiddenException"; | ||
| import { ExceptionMessage } from "../../constant/ExceptionMessage"; | ||
| import { Requester } from "../../infra/AuthTokenManager"; |
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.
보다보니 import 부분을 alias로 상대경로화 할 수 있지 않았을까 하네요.
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.
<맨 아래에서 DDD로 구성하셨다는 readme 보고 다시 올라와서 수정합니다.>
이 application 폴더에 대한 궁금증인데, 가만 보니 prisma 통해서 db 데이터와 연계하는 부분 아닌가요?
이걸 보통 비즈니스 로직이라 하는데, 이게 밑에서 domain 폴더에 있는 로직과 따로 떨어져있더군요.
어느정도 다시 재배합 하는게 필요해 보입니다.
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.
env는 constant에 넣지 않습니다. 루트에 놓습니다.
또한, 깃헙 레포에도 올리지 않습니다. 기본입니다.
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.
<맨 아래에서 DDD로 구성하셨다는 readme 보고 다시 올라와서 수정합니다.>
아까 위에서 작성한 application 폴더 안의 소스들과 적절히 배합해서 분할하는게 어떤가요?
getter, setter를 생각하시고 구성하신 것 같은데, 사실 express에서는 그렇게까지 안하셔도 됩니다..
nest.js에서는 이야기가 다르지만요.
| @@ -0,0 +1,19 @@ | |||
| /** | |||
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.
superstruct를 찾아보니 js api의 유효성검사를 위한 라이브러리군요. 기술 도입은 잘 하셨습니다.
그런데 왜 위에서는 도메인 별로 묶으셨는데 여기서는 안묶으셨나요?
파일이 많아지면 그만큼 컴파일링/트랜스파일링이 오래걸립니다. 서버에도 안좋아요.
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.
인증 미들웨어 위치는 interface/middleware 로 폴더를 따로 구성하심이 어떨까요.
요구사항
기본
공통
백엔드 배포
프로젝트 구조 및 환경 설정
AWS S3를 이용한 파일 업로드 시스템 구축
AWS RDS를 사용한 데이터베이스 관리
AWS EC2에서의 애플리케이션 운영
백엔드 테스트 구현
심화
테스트 구현
상품 이미지 업로드
AWS Route 53을 활용한 도메인 관리
SSL 인증서를 통한 HTTPS 연결 구현
주요 변경사항
멘토에게