IT Knowledge/Development/images/코드-리뷰-베스트-프랙티스-diagram.svg
코드 리뷰 베스트 프랙티스
📌 핵심 목적
쉬운 비유: 작가가 책을 쓴 후 편집자에게 검토받는 것. 오탈자 찾기보다는 “독자가 이해하기 쉬운가?”, “더 나은 표현은?” 같은 피드백이 중요합니다.
코드 리뷰의 목적:
- 버그 조기 발견
- 코드 품질 향상
- 지식 공유
- 팀 코딩 스타일 통일
🎯 리뷰어 가이드
1. 건설적인 피드백
❌ 나쁜 리뷰:
이 코드 왜 이렇게 짰어요? 너무 이상한데요.
✅ 좋은 리뷰:
현재 코드는 O(n²) 복잡도를 가지고 있네요.
Set을 사용하면 O(n)으로 개선할 수 있을 것 같습니다.
// 제안
const uniqueIds = new Set(users.map(u => u.id));
2. 코드 예시와 함께
❌ 모호한 피드백:
이 함수는 너무 길어요. 분리하세요.
✅ 구체적 제안:
// 현재 코드
function processOrder(order) {
// 100줄의 로직...
// 재고 확인
// 결제 처리
// 이메일 발송
// 포인트 적립
}
// 제안: 단일 책임 원칙 적용
function processOrder(order) {
validateStock(order);
const payment = processPayment(order);
sendConfirmationEmail(order, payment);
addLoyaltyPoints(order.userId, order.totalAmount);
}
function validateStock(order) {
// ...
}
function processPayment(order) {
// ...
}3. 우선순위 표시
// 🔴 Critical: 보안 취약점
// 현재 코드에서 SQL 인젝션 가능합니다.
const query = `SELECT * FROM users WHERE id = ${userId}`;
// 수정 필요:
const query = 'SELECT * FROM users WHERE id = ?';
db.query(query, [userId]);
// 🟡 Suggestion: 성능 개선 제안
// map().filter()를 한 번에 처리하면 더 효율적입니다.
const activeUsers = users
.filter(u => u.isActive)
.map(u => ({ id: u.id, name: u.name }));
// 💡 Nit: 사소한 스타일 개선
// 화살표 함수로 간결하게
- function getName() { return this.name; }
+ const getName = () => this.name;4. 칭찬도 중요
// ✅ 좋은 네이밍입니다!
function calculateTotalWithTax(subtotal, taxRate) {
return subtotal * (1 + taxRate);
}
// 👍 에러 처리가 잘 되어 있네요!
try {
await processPayment(order);
} catch (error) {
logger.error('Payment failed', { orderId: order.id, error });
await sendFailureNotification(order.userId);
throw error;
}👨💻 작성자 가이드
1. 작은 PR
❌ 나쁜 PR:
PR #123: "프로젝트 리팩토링"
- 50개 파일 수정
- 2,000줄 변경
- 리뷰 시간: 2시간+
✅ 좋은 PR:
PR #123: "User 모델에 이메일 검증 추가"
- 3개 파일 수정
- 50줄 변경
- 리뷰 시간: 10분
2. 명확한 PR 설명
## PR 제목
[Feature] 사용자 프로필 이미지 업로드 기능 추가
## 변경 내용
- 프로필 이미지 업로드 API 엔드포인트 추가
- S3에 이미지 저장
- 썸네일 자동 생성 (200x200)
## 테스트 방법
1. POST /api/users/123/avatar에 이미지 업로드
2. GET /api/users/123에서 avatarUrl 확인
3. S3 버킷에서 원본 + 썸네일 생성 확인
## 스크린샷
[이미지 첨부]
## 관련 이슈
Closes #4563. Self-Review
// PR 제출 전 스스로 리뷰
// ✅ 체크리스트
// - [ ] 테스트 통과
// - [ ] console.log 제거
// - [ ] 주석 작성 (복잡한 로직만)
// - [ ] 불필요한 코드 삭제
// - [ ] 포맷팅 (Prettier)
// - [ ] 린트 오류 없음
// ❌ 제거할 코드 예시
// console.log('디버깅:', user); // 삭제 필요
// const oldFunction = () => { }; // 사용 안 하는 코드🔧 자동화 도구
1. ESLint (코드 스타일)
// .eslintrc.js
module.exports = {
rules: {
'no-console': 'error', // console.log 금지
'no-unused-vars': 'error', // 사용 안 하는 변수 금지
'eqeqeq': 'error', // === 사용 강제
'max-len': ['error', { code: 100 }] // 한 줄 최대 100자
}
};2. Prettier (포맷팅)
// .prettierrc
{
"semi": true,
"singleQuote": true,
"tabWidth": 2,
"trailingComma": "es5"
}3. 커밋 전 자동 검사
// package.json
{
"husky": {
"hooks": {
"pre-commit": "lint-staged"
}
},
"lint-staged": {
"*.js": [
"eslint --fix",
"prettier --write",
"jest --findRelatedTests"
]
}
}💡 리뷰 문화
1. 리뷰 시간
긴급도에 따라:
- 🔴 Critical (보안, 장애): 1시간 이내
- 🟡 Normal: 24시간 이내
- 🟢 Low Priority: 48시간 이내
2. 리뷰 템플릿
## 🟢 잘된 점
- 에러 처리가 꼼꼼합니다
- 테스트 커버리지 95%
## 🟡 개선 제안
- [ ] 함수명을 더 명확하게 (getUserData → fetchUserProfile)
- [ ] 중복 코드 제거 가능 (L23-L45, L67-L89)
## 🔴 반드시 수정
- [ ] SQL 인젝션 취약점 (L156)
## ❓ 질문
- 이 로직에서 edge case는 어떻게 처리하나요?
(예: 사용자가 없는 경우)