|
| 1 | +--- |
| 2 | +name: review-pr |
| 3 | +description: Pull Request를 체계적으로 리뷰하여 프로젝트 컨벤션 준수 여부와 코드 품질을 검증합니다 |
| 4 | +args: <PR 번호> (예: /review-pr 666) |
| 5 | +--- |
| 6 | + |
| 7 | +# Pull Request 리뷰 가이드 |
| 8 | + |
| 9 | +이 skill은 solid-connect-server 프로젝트의 Pull Request를 체계적으로 리뷰합니다. |
| 10 | + |
| 11 | +## 사용법 |
| 12 | + |
| 13 | +```bash |
| 14 | +/review-pr <PR번호> |
| 15 | +``` |
| 16 | + |
| 17 | +**예제:** |
| 18 | +```bash |
| 19 | +/review-pr 666 |
| 20 | +``` |
| 21 | + |
| 22 | +--- |
| 23 | + |
| 24 | +## 리뷰 프로세스 |
| 25 | + |
| 26 | +### 1단계: PR 정보 수집 |
| 27 | + |
| 28 | +GitHub CLI로 PR의 기본 정보와 변경사항을 파악합니다. |
| 29 | + |
| 30 | +```bash |
| 31 | +gh pr view <번호> -R solid-connection/solid-connect-server # PR 기본 정보 조회 |
| 32 | +gh pr diff <번호> -R solid-connection/solid-connect-server # 변경된 파일과 diff 확인 |
| 33 | +gh pr checks <번호> -R solid-connection/solid-connect-server # CI/CD 상태 확인 |
| 34 | +``` |
| 35 | + |
| 36 | +**수집할 정보:** |
| 37 | +- PR 제목 및 설명 |
| 38 | +- 관련 이슈 번호 |
| 39 | +- 변경된 파일 목록 |
| 40 | +- CI/CD 체크 상태 |
| 41 | + |
| 42 | +### 2단계: 변경 파일 분석 |
| 43 | + |
| 44 | +**도구 우선순위:** |
| 45 | + |
| 46 | +1. **Serena MCP** (Java 코드 분석에 최적화) |
| 47 | + - `mcp__serena__get_symbols_overview <파일경로>` - 파일의 클래스/메서드 구조 파악 |
| 48 | + - `mcp__serena__find_symbol <심볼명>` - 특정 심볼 검색 |
| 49 | + - `mcp__serena__search_for_pattern <패턴>` - 컨벤션 위반 패턴 검색 |
| 50 | + |
| 51 | +2. **Read/Grep** (보조 분석) |
| 52 | + - `Read <파일경로>` - 파일 전체 읽기 |
| 53 | + - `Grep --pattern <패턴>` - 패턴 검색 |
| 54 | + |
| 55 | +### 3단계: 체크리스트 검증 |
| 56 | + |
| 57 | +아래 체크리스트를 순서대로 확인합니다. |
| 58 | + |
| 59 | +--- |
| 60 | + |
| 61 | +## 리뷰 체크리스트 |
| 62 | + |
| 63 | +각 항목의 상세 컨벤션은 참조 문서를 확인하세요. |
| 64 | + |
| 65 | +### 1. 아키텍처 및 계층 구조 |
| 66 | + |
| 67 | +**검증 항목:** |
| 68 | +- 계층형 아키텍처 준수 (Controller → Service → Repository) |
| 69 | +- 역계층 참조 금지 |
| 70 | +- 순환 의존성 없음 |
| 71 | + |
| 72 | +👉 **참고:** `CLAUDE.md` - "아키텍처" 섹션 |
| 73 | + |
| 74 | +--- |
| 75 | + |
| 76 | +### 2. 네이밍 컨벤션 |
| 77 | + |
| 78 | +**검증 항목:** |
| 79 | +- API 엔드포인트: kebab-case 사용 (예: `/user-profile`) |
| 80 | +- DTO 변환 메서드: 단일 파라미터 `from()`, 다중 파라미터 `of()` |
| 81 | +- Request/Response: `XXXRequest`, `XXXResponse` 형식 |
| 82 | +- 테스트 메서드: 한국어, `어떤_것을_하면_어떤_결과가_나온다()` 패턴 |
| 83 | + |
| 84 | +👉 **참고:** `CLAUDE.md` - "네이밍 컨벤션" 섹션 |
| 85 | + |
| 86 | +--- |
| 87 | + |
| 88 | +### 3. 코드 스타일 |
| 89 | + |
| 90 | +**검증 항목:** |
| 91 | +- 와일드카드(`*`) import 금지 |
| 92 | +- 클래스 선언 전 빈 줄 존재 |
| 93 | +- private 메서드는 호출하는 public 메서드 바로 아래 위치 |
| 94 | +- Controller: 모든 파라미터 줄바꿈 필수 |
| 95 | +- 일반 메서드: 3개 이상 파라미터 시 줄바꿈 |
| 96 | +- 파일 끝 개행 문자 |
| 97 | + |
| 98 | +**패턴 검색 예제:** |
| 99 | +```bash |
| 100 | +mcp__serena__search_for_pattern "import.*\\*" # 와일드카드 import 검색 |
| 101 | +``` |
| 102 | + |
| 103 | +👉 **참고:** `CLAUDE.md` - "코드 스타일" 섹션 |
| 104 | + |
| 105 | +--- |
| 106 | + |
| 107 | +### 4. Entity 및 JPA |
| 108 | + |
| 109 | +**검증 항목:** |
| 110 | +- 모든 필드에 `@Column` 어노테이션 존재 |
| 111 | +- `name` 속성으로 컬럼명 명시 |
| 112 | +- `nullable` 속성 명시 |
| 113 | +- null 불가: 원시 타입 (`int`, `long`, `boolean`) |
| 114 | +- nullable: Wrapper 타입 (`Integer`, `Long`, `Boolean`) |
| 115 | +- 양방향 연관관계 시 편의 메서드 존재 |
| 116 | + |
| 117 | +👉 **참고:** `CLAUDE.md` - "데이터베이스 접근" 섹션 |
| 118 | + |
| 119 | +--- |
| 120 | + |
| 121 | +### 5. 데이터베이스 마이그레이션 |
| 122 | + |
| 123 | +**검증 항목:** |
| 124 | +- 스키마 변경 시 Flyway 마이그레이션 파일 추가 |
| 125 | +- 파일명 형식: `V{VERSION}__{DESCRIPTION}.sql` |
| 126 | +- 위치: `src/main/resources/db/migration/` |
| 127 | +- Entity 변경과 마이그레이션 일치 |
| 128 | +- 기존 마이그레이션 파일 수정 금지 (새 버전 생성) |
| 129 | + |
| 130 | +👉 **참고:** `CLAUDE.md` - "데이터베이스 마이그레이션" 섹션 |
| 131 | + |
| 132 | +--- |
| 133 | + |
| 134 | +### 6. 테스트 코드 |
| 135 | + |
| 136 | +**검증 항목:** |
| 137 | +- 새로운 Service/Repository 메서드에 대한 테스트 존재 |
| 138 | +- 예외 케이스 테스트 포함 |
| 139 | +- `@TestContainerSpringBootTest` 어노테이션 사용 |
| 140 | +- `@DisplayName`으로 한글 설명 제공 |
| 141 | +- `@Nested`로 기능별 그룹화 |
| 142 | +- Given-When-Then 구조 준수 |
| 143 | +- Fixture 패턴 사용 (FixtureBuilder + Fixture) |
| 144 | + |
| 145 | +👉 **참고:** `.claude/skills/test/SKILL.md` |
| 146 | + |
| 147 | +--- |
| 148 | + |
| 149 | +### 7. 커밋 메시지 |
| 150 | + |
| 151 | +**검증 항목:** |
| 152 | +- `<type>: <description>` 형식 |
| 153 | +- Type: `feat`, `fix`, `refactor`, `test`, `chore`, `docs`, `perf` |
| 154 | +- 간결하고 명확한 설명 |
| 155 | + |
| 156 | +👉 **참고:** `CLAUDE.md` - "Git 커밋 컨벤션" 섹션 |
| 157 | + |
| 158 | +--- |
| 159 | + |
| 160 | +### 8. 코드 품질 및 잠재적 이슈 |
| 161 | + |
| 162 | +**검증 항목:** |
| 163 | +- 비즈니스 로직은 Service 계층에만 |
| 164 | +- Controller는 요청/응답 처리만 |
| 165 | +- `@Transactional` 적절하게 사용 (읽기 전용: `readOnly = true`) |
| 166 | +- CustomException 사용 |
| 167 | +- N+1 쿼리 문제 없음 |
| 168 | +- 인증/인가 처리 (`@AuthorizedUser`) |
| 169 | +- 민감 정보 노출 없음 |
| 170 | + |
| 171 | +👉 **참고:** `CLAUDE.md` - "아키텍처", "기술 스택 상세" 섹션 |
| 172 | + |
| 173 | +--- |
| 174 | + |
| 175 | +## 도구 사용 가이드 |
| 176 | + |
| 177 | +### Serena MCP (우선 사용) |
| 178 | + |
| 179 | +```bash |
| 180 | +# 파일의 클래스/메서드 구조 파악 |
| 181 | +mcp__serena__get_symbols_overview src/main/java/.../MentorService.java |
| 182 | + |
| 183 | +# 특정 심볼 검색 |
| 184 | +mcp__serena__find_symbol "MentorDetailResponse" |
| 185 | + |
| 186 | +# 컨벤션 위반 패턴 검색 |
| 187 | +mcp__serena__search_for_pattern "import.*\\*" |
| 188 | +``` |
| 189 | + |
| 190 | +### GitHub CLI |
| 191 | + |
| 192 | +```bash |
| 193 | +# PR 정보 |
| 194 | +gh pr view 666 -R solid-connection/solid-connect-server --json title,body,author,number,url |
| 195 | + |
| 196 | +# 변경사항 |
| 197 | +gh pr diff 666 -R solid-connection/solid-connect-server --patch |
| 198 | + |
| 199 | +# CI 상태 |
| 200 | +gh pr checks 666 -R solid-connection/solid-connect-server |
| 201 | +``` |
| 202 | + |
| 203 | +### 보조 도구 |
| 204 | + |
| 205 | +```bash |
| 206 | +# 파일 읽기 |
| 207 | +Read src/main/java/.../MentorService.java |
| 208 | + |
| 209 | +# 패턴 검색 |
| 210 | +Grep --pattern "@Column" --glob "*.java" --path src/main/java/.../domain |
| 211 | +``` |
| 212 | + |
| 213 | +--- |
| 214 | + |
| 215 | +## 리뷰 결과 출력 형식 |
| 216 | + |
| 217 | +다음 형식으로 리뷰 결과를 정리하여 제공합니다. |
| 218 | + |
| 219 | +```markdown |
| 220 | +## PR 리뷰 결과: #{번호} - {제목} |
| 221 | + |
| 222 | +**PR 링크:** {GitHub URL} |
| 223 | +**관련 이슈:** #{이슈번호} |
| 224 | + |
| 225 | +### 📊 PR 정보 요약 |
| 226 | + |
| 227 | +- **작성자:** {작성자} |
| 228 | +- **변경 파일:** {숫자}개 |
| 229 | +- **추가 라인:** +{숫자}, **삭제 라인:** -{숫자} |
| 230 | +- **CI/CD 상태:** {통과/실패} |
| 231 | + |
| 232 | +### 주요 변경사항 |
| 233 | + |
| 234 | +{PR 설명 요약} |
| 235 | + |
| 236 | +--- |
| 237 | + |
| 238 | +### ✅ 통과 항목 |
| 239 | + |
| 240 | +- 아키텍처 계층 구조 준수 |
| 241 | +- 네이밍 컨벤션 준수 |
| 242 | +- ... |
| 243 | + |
| 244 | +### ⚠️ 개선 권장 항목 |
| 245 | + |
| 246 | +- **코드 스타일**: 와일드카드 import 사용 |
| 247 | + - 파일: `src/main/java/.../MentorService.java:5` |
| 248 | + - 개선: 명시적 import로 변경 |
| 249 | + |
| 250 | +### ❌ 필수 수정 항목 |
| 251 | + |
| 252 | +- **Entity**: @Column 어노테이션 누락 |
| 253 | + - 파일: `src/main/java/.../domain/Mentor.java:30` |
| 254 | + - 수정 방향: 모든 필드에 `@Column` 어노테이션 추가 |
| 255 | + |
| 256 | +--- |
| 257 | + |
| 258 | +### 💡 종합 의견 |
| 259 | + |
| 260 | +{전반적인 리뷰 의견} |
| 261 | + |
| 262 | +**승인 상태:** ✅ 승인 / ⚠️ 조건부 승인 / ❌ 수정 후 재검토 |
| 263 | +``` |
| 264 | + |
| 265 | +--- |
| 266 | + |
| 267 | +## 리뷰 시 주의사항 |
| 268 | + |
| 269 | +1. **컨텍스트 이해 우선**: PR 설명과 관련 이슈를 먼저 읽고 변경의 목적 파악 |
| 270 | +2. **Serena MCP 우선 사용**: Java 코드 분석 시 효율적 |
| 271 | +3. **건설적 피드백**: 문제점 지적 시 구체적인 개선 방향 제시 |
| 272 | +4. **긍정적 피드백**: 잘된 부분도 언급하여 균형 잡힌 리뷰 |
| 273 | +5. **우선순위**: 아키텍처 > 네이밍 > 스타일 순으로 중요도 판단 |
| 274 | + |
| 275 | +--- |
| 276 | + |
| 277 | +## 참고 자료 |
| 278 | + |
| 279 | +- **프로젝트 컨벤션**: `CLAUDE.md` - 전체 개발 컨벤션 |
| 280 | +- **테스트 가이드**: `.claude/skills/test/SKILL.md` - 테스트 작성 가이드 |
| 281 | +- **개발 컨벤션 위키**: https://github.com/solid-connection/solid-connect-server/wiki/개발-컨벤션-정리 |
0 commit comments