chore: pr을 리뷰하는 SKILL.md 작성#669
Merged
whqtker merged 2 commits intosolid-connection:developfrom Feb 19, 2026
Merged
Conversation
whqtker
requested review from
Gyuhyeok99,
Hexeong,
JAEHEE25,
lsy1307,
sukangpunch and
wibaek
as code owners
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 제안된 리뷰어
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
Comment on lines
+28
to
+34
| GitHub CLI로 PR의 기본 정보와 변경사항을 파악합니다. | ||
|
|
||
| ```bash | ||
| gh pr view <번호> -R solid-connection/solid-connect-server # PR 기본 정보 조회 | ||
| gh pr diff <번호> -R solid-connection/solid-connect-server # 변경된 파일과 diff 확인 | ||
| gh pr checks <번호> -R solid-connection/solid-connect-server # CI/CD 상태 확인 | ||
| ``` |
There was a problem hiding this comment.
-
- 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.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.claude/hooks/notify.py (1)
16-29: 전반적인 폴백 로직은 잘 구현되었습니다. 아래 선택적 개선 사항을 참고해 주세요.
✅ 전체 접근 방식 승인
- VS Code 확장 환경에서 PATH에
powershell이 없을 수 있다는 상황을 절대 경로 우선 시도로 깔끔하게 해결했습니다.timeout=10및FileNotFoundError/TimeoutExpired포착을 통해 무한 대기 없이 폴백이 동작하는 구조가 좋습니다.💡 (선택)
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🔇 정적 분석 경고 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
관련 이슈
작업 내용
클로드의
review-pr <PR번호>를 통해 PR을 리뷰하는 SKILL.md를 작성했습니다. 로컬에 Github CLI가 설치되어 있다고 가정하고 작성했습니다(설치되어 있지 않아도 잘 동작하긴 할 것 같습니다).PR 타이틀과 디스크립션을 읽기에, 자세히 작성해주시면 더 잘 동작합니다 !!
아래 대화는 실제로 해당 SKILL.md를 통해 #656 을 리뷰한 내용입니다.
한 번 사용해보시고, 개선할 점 있으면 의견 부탁드리겠습니다 !!
추가로 claude noti가 윈도우에서 여전히 안 와서 해당 내용도 수정했습니다. vscode claude extension 사용 시 알림은 webview 형식이라 알림이 따로 뜨진 않네요 cli에서 구동하는 경우는 잘 동작합니다 ..!
특이 사항
리뷰 요구사항 (선택)