-
Notifications
You must be signed in to change notification settings - Fork 0
[24.11.24 / TASK-47] Feature - 프로젝트 추가 Init #3
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
Conversation
Co-authored-by: 이호준 <akk808@snu.ac.kr>
Walkthrough이 풀 리퀘스트에서는 여러 파일에서 환경 변수 및 코드 구조의 변경이 이루어졌습니다. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (2)src/app.ts (2)
이전 리뷰에서 논의된 대로 기본 포트를 8080으로 변경해야 합니다. 다음과 같이 수정해주세요: app.use(cors({
- origin: process.env.ALLOWED_ORIGINS?.split(',') || 'http://localhost:3000',
+ origin: process.env.ALLOWED_ORIGINS?.split(',') || 'http://localhost:8080',
methods: ['GET', 'POST', 'PUT', 'DELETE', 'OPTIONS'],
allowedHeaders: ['Content-Type', 'Authorization'],
credentials: true
}));환경 변수에도 다음을 추가해주세요: Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (4)
src/types/PostDailyStatistics.type.ts (2)
1-9: 기본 구조는 적절하나 타입 개선이 필요합니다.인터페이스의 기본 구조는 잘 정의되어 있습니다. 하지만
date필드의 타입을string대신Date로 변경하는 것이 더 적절할 것 같습니다. 이는 날짜 관련 연산과 검증을 더 안전하게 수행할 수 있게 해줍니다.다음과 같이 변경을 제안합니다:
export interface PostDailyStatistics { id: number; postId: number; - date: string; + date: Date; dailyViewCount: number; dailyLikeCount: number; createdAt: Date; updatedAt: Date; }
1-9: 문서화 주석 추가를 제안합니다.인터페이스와 각 필드에 대한 JSDoc 문서화 주석을 추가하면 코드의 가독성과 유지보수성이 향상될 것 같습니다.
다음과 같이 문서화 주석을 추가하는 것을 제안합니다:
+ /** + * 게시물의 일일 통계 정보를 나타내는 인터페이스 + */ export interface PostDailyStatistics { + /** 통계 기록의 고유 식별자 */ id: number; + /** 연관된 게시물의 식별자 */ postId: number; + /** 통계가 집계된 날짜 */ date: string; + /** 해당 일자의 조회수 */ dailyViewCount: number; + /** 해당 일자의 좋아요 수 */ dailyLikeCount: number; + /** 통계 기록 생성 시간 */ createdAt: Date; + /** 통계 기록 최종 수정 시간 */ updatedAt: Date; }src/types/User.type.ts (1)
1-11: 선택적 필드 지원 및 문서화 개선 제안인터페이스의 모든 필드가 필수로 지정되어 있어, 부분적인 데이터 처리가 어려울 수 있습니다.
다음과 같은 개선을 제안드립니다:
+/** + * 사용자 정보를 나타내는 인터페이스 + */ export interface User { id: number; velog_uuid: string; access_token: string; refresh_token: string; group_id: number; email: string; - is_active: boolean; + is_active?: boolean; - created_at: Date; + created_at?: Date; - updated_at: Date; + updated_at?: Date; }src/app.ts (1)
11-13: 루트 경로의 응답이 너무 단순합니다.API 상태 확인을 위해서는 좀 더 유용한 정보를 제공하는 것이 좋습니다.
다음과 같이 수정하는 것을 제안드립니다:
-app.get('/', (req, res) => { - res.send('Hello, V.D.!'); -}); +app.get('/', (req, res) => { + res.json({ + status: 'ok', + version: process.env.npm_package_version, + timestamp: new Date().toISOString() + }); +});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.env.sample(1 hunks).pre-commit-config.yaml(1 hunks)README.md(1 hunks)package.json(2 hunks)src/app.ts(1 hunks)src/types/Post.type.ts(1 hunks)src/types/PostDailyStatistics.type.ts(1 hunks)src/types/PostStatistics.type.ts(1 hunks)src/types/User.type.ts(1 hunks)src/types/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- README.md
- src/types/Post.type.ts
- src/types/PostStatistics.type.ts
- src/types/index.ts
🔇 Additional comments (6)
.env.sample (1)
7-16:
AES 키의 보안 관련 우려사항
빈 문자열로 설정된 AES 키는 보안상 위험할 수 있습니다. 프로덕션 환경에서는 반드시 강력한 암호화 키를 사용해야 합니다.
다음 스크립트를 실행하여 AES 키 사용 여부를 확인하겠습니다:
src/app.ts (1)
3-3: CORS 패키지가 올바르게 임포트되었습니다.
필요한 CORS 기능을 위한 패키지 임포트가 적절히 이루어졌습니다.
.pre-commit-config.yaml (2)
1-11: 원격 pre-commit 훅 구성이 적절합니다!
선택된 훅들이 코드 품질 관리에 필수적인 요소들을 포함하고 있습니다:
- 공백 및 줄 끝 문자 정리
- YAML 파일 검증
- 대용량 파일 감지
- 충돌 및 보안 관련 검사
1-20: pre-commit 설정의 종속성 확인이 필요합니다.
package.json에 필요한 개발 종속성이 모두 명시되어 있는지 확인이 필요합니다.
✅ Verification successful
pre-commit 설정이 올바르게 구성되어 있습니다.
package.json에 필요한 모든 개발 종속성이 정확하게 명시되어 있습니다:
- lint-staged (v15.2.10)
- prettier (v3.3.3)
- eslint (v9.15.0)과 관련 플러그인들
또한 lint-staged 설정도 적절하게 구성되어 있어 TypeScript 파일들에 대해 eslint와 prettier가 실행됩니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# package.json에서 필요한 개발 종속성 확인
# lint-staged 설정 확인
jq '.["lint-staged"]' package.json
# 개발 종속성 확인
jq '.devDependencies | select(has("lint-staged") or has("prettier") or has("eslint"))' package.json
Length of output: 750
package.json (2)
8-8: 스크립트 구성이 개선되었습니다.
TypeScript 빌드 프로세스와 린트 설정이 더 명확하게 구성되었습니다:
build스크립트가 TypeScript 컴파일러를 직접 사용하도록 변경lint:fix명령이 src/ 디렉토리를 대상으로 하도록 구체화
Also applies to: 10-10
16-21: lint-staged 설정이 적절히 구성되었습니다.
코드 품질 관리를 위한 lint-staged 설정이 추가되었습니다. TypeScript 파일에 대해 ESLint와 Prettier를 순차적으로 실행하는 구성이 적절합니다.
Nuung
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.
특이 사항 없습니다! 고생하셨습니다!
좋았던 점
- 전체 프로젝트 init 과 디테일에 신경을 다시 한 번 썻다는 것을 느꼈습니다! (린팅에서도 ㅎㅎ)
- pre-commit-hook 종류가 살벌합니다. 이거 커밋하기 무섭네요
궁금해요!
- 궁금한게 있습니다! types 외에 repository layer 에서는 model 을 어떻게 사용하실지 궁금합니다!
BDlhj
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.
PR 잘 확인했습니다!!
여러 번 보다가 갑자기 눈에 띄는 부분이 생겨서 코멘트 남겨두었습니다.
혹시 제가 착각한 거면 바로 말씀주십쇼!! 바~~~로 approve 드리겠습니다!!
좋았던 점
pre-commit이 추가되면서 프로젝트 세팅이 더 좋아진 것 같습니다. 현우님 말씀처럼 체크하는 것들이 많지만 그만큼 앞으로 안정적으로 코드를 관리할 수 있지 않을까 싶어요!! back office 쪽에도 몇 가지를 더 추가해야하나 생각해봐야겠습니다😊
|
@Nuung 아직 작성한 건 아니지만 아마 아래와 같이 사용할 거 같습니다 모델링이 필요하다면 pg의 마이그레이션 라이브러리나 쿼리문을 ddl로 만들어 함수로 실행 되게 할 거 같습니다 |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@BDlhj @six-standard 확인 되셨으면 approve 부탁드립니다! 이틀 됐습니다.. 이제 그만.. 보내주세요... |
앗?! 제가 re-request 보내신 걸 놓쳤었나봅니다.. 지금 바로 approve 드리겠습니다~~ |
six-standard
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.
엇 죄송합니다.. 시험 기간동안 PR이 좀 많이 밀린 것 같은데 확인하질 못했습니다;
좋았던 점
CORS가 미리 설정되어서 좋았습니다 (연동 도중에 CORS 터질 일이 없어서 좋네요)
pre-commit 통해서 코드 컨벤션에 맞게 훨씬 안정적으로 커밋할 수 있게 바뀐 점이 좋았습니다.
Summary by CodeRabbit
New Features
User,Post,PostDailyStatistics,PostStatistics)가 추가되었습니다.Bug Fixes
.env.sample파일의 AES 키 값이 초기화되어 보안성을 향상시켰습니다.Documentation
README.md파일의 설치 주석이 수정되었습니다.Chores
package.json파일에 CORS 관련 의존성과 lint-staged가 추가되었습니다.