chore: pr을 리뷰하는 SKILL.md 작성

[whqtker]

관련 이슈

• resolves: chore: PR 리뷰를 담당하는 SKILL.md 작성 #668

작업 내용

클로드의 review-pr <PR번호> 를 통해 PR을 리뷰하는 SKILL.md를 작성했습니다. 로컬에 Github CLI가 설치되어 있다고 가정하고 작성했습니다(설치되어 있지 않아도 잘 동작하긴 할 것 같습니다).

PR 타이틀과 디스크립션을 읽기에, 자세히 작성해주시면 더 잘 동작합니다 !!

아래 대화는 실제로 해당 SKILL.md를 통해 #656 을 리뷰한 내용입니다.

/review-pr 656

---

## Claude 응답

PR #656을 리뷰하겠습니다. 먼저 PR 정보를 수집하고 변경사항을 분석하겠습니다.

### PR 정보 수집 결과

**기본 정보:**
- PR 번호: #656
- 제목: feat: 어드민 국가 crud 추가
- 작성자: Gyuhyeok99 (황규혁)
- 관련 이슈: #584
- 추가 라인: +462, 삭제 라인: -0
- 변경 파일: 9개

**CI/CD 상태:**
- build: ❌ fail (1m55s)
- CodeRabbit: ✅ pass

---

## PR 리뷰 결과: #656 - feat: 어드민 국가 crud 추가

**PR 링크:** https://github.com/solid-connection/solid-connect-server/pull/656
**관련 이슈:** #584
**작성자:** Gyuhyeok99 (황규혁)

### 📊 PR 정보 요약

- **변경 파일:** 9개
- **추가 라인:** +462, **삭제 라인:** -0
- **CI/CD 상태:** ❌ 실패 (TestContainers Docker 초기화 오류)

### 주요 변경사항

어드민 국가 관련 CRUD 기능을 추가했습니다.

**새로 추가된 파일:**
- [AdminCountryController.java](src/main/java/com/example/solidconnection/admin/location/country/controller/AdminCountryController.java) - 국가 CRUD API 엔드포인트
- [AdminCountryService.java](src/main/java/com/example/solidconnection/admin/location/country/service/AdminCountryService.java) - 비즈니스 로직
- [AdminCountryCreateRequest.java](src/main/java/com/example/solidconnection/admin/location/country/dto/AdminCountryCreateRequest.java) - 생성 요청 DTO
- [AdminCountryUpdateRequest.java](src/main/java/com/example/solidconnection/admin/location/country/dto/AdminCountryUpdateRequest.java) - 수정 요청 DTO
- [AdminCountryResponse.java](src/main/java/com/example/solidconnection/admin/location/country/dto/AdminCountryResponse.java) - 응답 DTO
- [AdminCountryServiceTest.java](src/test/java/com/example/solidconnection/admin/location/country/service/AdminCountryServiceTest.java) - 서비스 테스트

**수정된 파일:**
- [Country.java](src/main/java/com/example/solidconnection/location/country/domain/Country.java) - 업데이트 메서드 추가
- [ErrorCode.java](src/main/java/com/example/solidconnection/common/exception/ErrorCode.java) - `COUNTRY_ALREADY_EXISTS` 추가

---

### ✅ 통과 항목

- **아키텍처 계층 구조 준수**: Controller → Service → Repository 패턴 준수
- **네이밍 컨벤션 준수**:
  - API 엔드포인트 kebab-case 사용 (`/admin/countries`)
  - DTO 네이밍 (`XXXRequest`, `XXXResponse`)
  - DTO 변환 메서드 (`from()`)
  - 테스트 메서드 한국어 네이밍
- **Entity 적절히 작성**:
  - 기존 Country 엔티티에 모든 필드에 `@Column` 존재
  - nullable 속성 명시
  - 업데이트 메서드 적절히 추가
- **데이터베이스 마이그레이션**: 컬럼 추가 없으므로 불필요 (올바름)
- **테스트 코드 완성도**:
  - 전체 CRUD 작업에 대한 테스트 존재
  - 예외 케이스 포함
  - `@TestContainerSpringBootTest` 사용
  - `@DisplayName`, `@Nested` 그룹화
  - Given-When-Then 구조
  - Fixture 패턴 사용
- **커밋 메시지**: `feat: 어드민 국가 crud 추가` (형식 준수)
- **코드 품질**:
  - `@Transactional` 적절히 사용 (`readOnly = true` 포함)
  - CustomException 사용
  - private 메서드가 호출하는 public 메서드 바로 아래 위치
  - 비즈니스 로직이 Service 계층에 위치
  - Controller는 요청/응답 처리만 담당

---

### ⚠️ 개선 권장 항목

#### **코드 스타일: import 순서 불일치**

**파일:**
- [AdminCountryController.java:19](src/main/java/com/example/solidconnection/admin/location/country/controller/AdminCountryController.java#L19)
- [AdminCountryService.java:15](src/main/java/com/example/solidconnection/admin/location/country/service/AdminCountryService.java#L15)

**문제:**
// 현재 (AdminCountryController.java)
import org.springframework.web.bind.annotation.RestController;

import java.util.List;  // ❌ 맨 마지막에 위치


**개선 방향:**
프로젝트 컨벤션에 따라 import 순서는 `jakarta → java → lombok → spring` 순이어야 합니다.

// 올바른 순서 (AdminRegionController.java 참고)
import jakarta.validation.Valid;
import java.util.List;  // ✅ java는 jakarta 다음, lombok 전
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;

**참고:** [AdminRegionController.java:7-9](src/main/java/com/example/solidconnection/admin/location/region/controller/AdminRegionController.java#L7-L9)

---

### 🔧 CI/CD 실패 분석

**실패 원인:** TestContainers Docker 초기화 실패

java.lang.ExceptionInInitializerError at DockerClientProviderStrategy.java:276


**분석:**
- 모든 테스트가 동일한 원인(`ExceptionInInitializerError`)으로 실패
- `AdminRegionServiceTest` 등 기존 테스트도 동일하게 실패
- 코드 품질 문제가 아닌 CI 환경의 Docker 설정 문제로 판단됨
- 테스트 코드 자체는 올바르게 작성됨

**권장 사항:**
- CI 환경의 Docker 설정 확인
- 또는 로컬에서 `./gradlew test` 실행하여 테스트 통과 확인

---

### 💡 종합 의견

이 PR은 AdminRegion 패턴을 일관되게 따라 AdminCountry CRUD 기능을 잘 구현했습니다.

**장점:**
- 기존 `AdminRegionController`/`AdminRegionService` 패턴을 일관되게 적용
- 계층형 아키텍처 준수
- 유효성 검증 로직 적절히 분리 (`validateXXX` private 메서드)
- 테스트 커버리지 우수 (모든 CRUD + 예외 케이스)
- 중복 체크 로직 세심함 (생성 시 코드/한글명 중복, 수정 시 자기 자신 제외 중복 체크)

**개선 사항:**
- import 순서를 프로젝트 컨벤션에 맞게 수정하면 완벽합니다.

**승인 상태:** ⚠️ **조건부 승인**
- import 순서 수정 후 머지 권장
- CI 실패는 인프라 문제로 보이므로 로컬 테스트 통과 확인 필요

한 번 사용해보시고, 개선할 점 있으면 의견 부탁드리겠습니다 !!

---

추가로 claude noti가 윈도우에서 여전히 안 와서 해당 내용도 수정했습니다. vscode claude extension 사용 시 알림은 webview 형식이라 알림이 따로 뜨진 않네요 cli에서 구동하는 경우는 잘 동작합니다 ..!

특이 사항

리뷰 요구사항 (선택)

[coderabbitai]

Walkthrough

1. .claude/skills/review-pr/SKILL.md 파일을 새로 추가하여 solid-connect-server용 PR 리뷰 워크플로와 도구 사용법, 체크리스트, 출력 템플릿 등을 문서화했습니다.
2. .claude/hooks/notify.py를 수정하여 Windows에서 PowerShell 실행 후보군을 순회하고 각 후보에 대해 타임아웃과 예외 처리를 적용하도록 변경했습니다.
3. 비-윈도우 플랫폼 기존 동작은 유지되며, 후보 중 하나가 성공하면 루프를 중단하도록 구현되었습니다.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

제안된 리뷰어

• wibaek
• Hexeong
• JAEHEE25
• lsy1307
• Gyuhyeok99

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	❓ Inconclusive	notify.py 파일 수정은 PR 설명의 '예정'이라는 언급과 불일치하며 범위 내 포함 여부가 명확하지 않습니다.	notify.py 변경사항이 이슈 #668과의 연관성을 명확히 하거나 별도 PR로 분리하는 것을 검토하세요.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	PR 제목은 주요 변경사항인 PR 리뷰 SKILL.md 파일 작성을 명확하게 설명하고 있습니다.
Description check	✅ Passed	PR 설명은 필수 섹션(관련 이슈, 작업 내용)을 완벽히 포함하고 실제 사용 예시까지 제공하고 있습니다.
Linked Issues check	✅ Passed	PR은 연결된 이슈 #668의 핵심 목표인 PR 리뷰용 SKILL.md 작성을 완벽히 충족하고 있습니다.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In @.claude/skills/review-pr/SKILL.md:
- Around line 28-34: Clarify that the GH CLI commands shown ("gh pr view", "gh
pr diff", "gh pr checks") require the GitHub CLI to be installed and
authenticated, or alternatively provide minimal non-CLI options (e.g., link to
the GitHub web PR view and mention using the REST API endpoints or curl for PR
details/diff/checks) so readers know installation is mandatory or can follow a
web/API fallback; update the text around the three commands to state the
dependency and include one-line guidance for a web UI link and one-line pointer
to the REST API alternative.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

• 1. GH CLI 의존성 명확화 또는 대안 경로 추가
  • 현재 단계 1은 gh 명령만 제시되어 있어, “설치 없이도 동작 가능”이라는 설명과 충돌할 소지가 있어요.
  • 설치가 필수인지 명확히 하거나, GitHub 웹 UI/API로 대체 가능한 최소 경로를 함께 안내하는 쪽이 사용자 혼란을 줄입니다.

📝 제안된 문서 보완(diff)

- GitHub CLI로 PR의 기본 정보와 변경사항을 파악합니다.
+ GitHub CLI로 PR의 기본 정보와 변경사항을 파악합니다.
+ (GH CLI 미설치 시) GitHub 웹 UI의 “Files changed / Checks” 탭 또는 REST API로 동일 정보를 확인할 수 있습니다.

🤖 Prompt for AI Agents

In @.claude/skills/review-pr/SKILL.md around lines 28 - 34, Clarify that the GH
CLI commands shown ("gh pr view", "gh pr diff", "gh pr checks") require the
GitHub CLI to be installed and authenticated, or alternatively provide minimal
non-CLI options (e.g., link to the GitHub web PR view and mention using the REST
API endpoints or curl for PR details/diff/checks) so readers know installation
is mandatory or can follow a web/API fallback; update the text around the three
commands to state the dependency and include one-line guidance for a web UI link
and one-line pointer to the REST API alternative.

[sukangpunch]

너무 좋네요 고생하셨습니다!

[coderabbitai]

🧹 Nitpick comments (1)

.claude/hooks/notify.py (1)

16-29: 전반적인 폴백 로직은 잘 구현되었습니다. 아래 선택적 개선 사항을 참고해 주세요.

1. ✅ 전체 접근 방식 승인

   • VS Code 확장 환경에서 PATH에 powershell이 없을 수 있다는 상황을 절대 경로 우선 시도로 깔끔하게 해결했습니다.
   • timeout=10 및 FileNotFoundError / TimeoutExpired 포착을 통해 무한 대기 없이 폴백이 동작하는 구조가 좋습니다.

2. 💡 (선택) break 조건 — 실행 성공 vs. 알림 성공 구분

   • 현재 구조에서는 subprocess.run이 예외 없이 반환되면(실행 파일을 찾아 실행은 했지만 종료 코드가 0이 아닌 경우 포함) 즉시 break됩니다.
   • 예를 들어 ps1_path가 존재하지 않으면 PowerShell이 실행은 되지만 오류를 내고 종료하며, 알림은 표시되지 않는데도 루프가 끝납니다.
   • 두 후보가 동일한 ps1_path를 사용하므로 현실적 영향은 없지만, 보다 견고하게 만들고 싶다면 아래와 같이 수정할 수 있습니다.
   ♻️ 선택적 개선 — check=True 및 CalledProcessError 포착

        for ps in powershell_candidates:
            try:
                subprocess.run(
                    [ps, "-NoProfile", "-ExecutionPolicy", "Bypass", "-File", ps1_path],
   -                timeout=10,
   +                timeout=10,
   +                check=True,
                )
                break
   -        except (FileNotFoundError, subprocess.TimeoutExpired):
   +        except (FileNotFoundError, subprocess.TimeoutExpired, subprocess.CalledProcessError):
                continue

3. 🔇 정적 분석 경고 S603 — 무시 가능

   • ps는 하드코딩된 리스트에서만 오고, ps1_path는 __file__ 기준의 경로이므로 외부 입력이 없습니다. Ruff S603은 여기서 거짓 양성(false positive) 입니다.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.claude/hooks/notify.py around lines 16 - 29, Update the subprocess
invocation in the powershell_candidates loop so that you only break on a
successful execution: call subprocess.run with check=True inside the loop (still
using timeout=10) and catch subprocess.CalledProcessError in addition to
FileNotFoundError and subprocess.TimeoutExpired; only break the loop when
subprocess.run returns without raising (i.e., the command exited with code 0).
Reference the powershell_candidates list, the loop that iterates over ps, and
the subprocess.run call to locate where to add check=True and the new exception
handling.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In @.claude/hooks/notify.py:
- Around line 16-29: Update the subprocess invocation in the
powershell_candidates loop so that you only break on a successful execution:
call subprocess.run with check=True inside the loop (still using timeout=10) and
catch subprocess.CalledProcessError in addition to FileNotFoundError and
subprocess.TimeoutExpired; only break the loop when subprocess.run returns
without raising (i.e., the command exited with code 0). Reference the
powershell_candidates list, the loop that iterates over ps, and the
subprocess.run call to locate where to add check=True and the new exception
handling.