-
-
Notifications
You must be signed in to change notification settings - Fork 658
add github PR review mechanics #41
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
base: main
Are you sure you want to change the base?
add github PR review mechanics #41
Conversation
WalkthroughAdded comprehensive documentation for GitHub PR mechanics in both the receiving-code-review and requesting-code-review skill directories. Created two new github-reviews.md files covering feedback handling for implementers and review posting for reviewers. Updated existing SKILL.md files to reference the new GitHub PR Mechanics section and corrected template paths. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
|
sorry, poor form to open this as a draft. Claude got out ahead of its skis. I'll keep it here while I test this though to keep noise low. |
|
Ok, a first couple comments:
|
|
Great. Yes this felt perhaps the wrong place to put this, and wanted this very discussion! Happy to move into its own markdown. |
…n of labor to prevent context pollution.
|
Getting meta: here is the code review against this PR: |
|
@obra I've cleaned this up, but things have changed more since I wrote this. Is this still the correct approach? |
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: 1
🧹 Nitpick comments (5)
skills/receiving-code-review/github-reviews.md (3)
1-3: Add ABOUTME/introductory context per project conventions.PR feedback noted that markdown reference files should include introductory ABOUTME comments per project conventions. This file lacks that context.
Consider adding a brief ABOUTME or context block after the title explaining file purpose, similar to:
> **ABOUTME:** This file provides command reference and operational guidance for implementers responding to code review feedback on GitHub PRs. It covers reading feedback, responding appropriately, and workflow details. For conceptual approach to code review reception, see `SKILL.md`.
64-71: Add error handling guidance for command substitution.Line 65-70 shows command chaining with
git apiqueries that could fail silently if the repository state or query changes. No error handling is shown for this scenario.Consider adding a note about handling command failures:
**Note:** If the API call fails (e.g., invalid PR number), the command will error. Verify the PR number and that you have push access before running.
78-84: Provide guidance on finding your own comment IDs for editing.The editing section (line 78-84) assumes users know their comment ID but provides no guidance on finding it.
Add a reference to retrieving your own comment:
**To find your comment ID:** Use the "Reading Review Feedback" section commands to fetch comments and locate your own by author/body text, then extract the `id` field.skills/requesting-code-review/github-reviews.md (2)
1-3: Add ABOUTME/introductory context per project conventions.PR feedback noted that markdown reference files should include introductory ABOUTME comments per project conventions. This file lacks that context.
Consider adding brief context after the title explaining file purpose:
> **ABOUTME:** This file provides command reference and operational guidance for code-reviewer agents analyzing PRs hosted on GitHub. It covers fetching PR content, existing discussion, and posting findings. For conceptual approach to code review, see parent `SKILL.md`.
73-89: Add error handling guidance for command substitution.Line 79 shows a command substitution for the headRefOid that could fail silently if the API call errors or returns empty. No guidance is provided for handling these failures.
Add error handling note:
**Note:** If `gh pr view` fails (invalid PR or insufficient access), the command will error. Verify the PR number exists and you have repository access before running. For debugging, remove the substitution and run `gh pr view <number> --json headRefOid -q .headRefOid` separately first.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
skills/receiving-code-review/SKILL.md(1 hunks)skills/receiving-code-review/github-reviews.md(1 hunks)skills/requesting-code-review/SKILL.md(1 hunks)skills/requesting-code-review/github-reviews.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
skills/requesting-code-review/github-reviews.md
[uncategorized] ~8-~8: The official name of this software platform is spelled with a capital “H”.
Context: ...uired:** 1. Repository remote hosted on github.com 2. GitHub CLI (gh) installed and ...
(GITHUB)
skills/receiving-code-review/github-reviews.md
[uncategorized] ~8-~8: The official name of this software platform is spelled with a capital “H”.
Context: ...uired:** 1. Repository remote hosted on github.com 2. GitHub CLI (gh) installed and ...
(GITHUB)
🔇 Additional comments (6)
skills/receiving-code-review/github-reviews.md (2)
1-4: Capitalize "GitHub" per style guidelines.Line 8 and other instances use lowercase "github.com"; use "GitHub" per official naming.
Apply this diff:
-1. Repository remote hosted on github.com +1. Repository remote hosted on GitHub.comAlso apply to line 42 in the API endpoint example (contains "github.com" in the URL path).
8-8: I'll help you verify the concerns raised in this review comment about the GitHub reviews documentation file. Let me start by examining the file content and then verify the GitHub API endpoints and CLI flag usage.
<function_calls>
cat -n skills/receiving-code-review/github-reviews.md | head -100
</function_calls>Let me search for the file first:
<function_calls>
fd "github-reviews.md" --type f
</function_calls>Let me get a broader search:
<function_calls>
find . -name "github-reviews.md" -type f 2>/dev/null
</function_calls>Let me check the directory structure:
<function_calls>
find . -type d -name "receiving-code-review" 2>/dev/null | head -5
</function_calls>Let me list what files exist in skills:
<function_calls>
git ls-files skills/ 2>/dev/null | head -20
</function_calls>Now let me verify the GitHub API documentation and gh CLI flag usage through web search:
<function_calls>
GitHub API POST /repos/{owner}/{repo}/pulls/comments/{comment_id}/replies endpoint
</function_calls><function_calls>
gh cli flag -f vs -F difference numeric parameters
</function_calls><function_calls>
gh api POST /repos/pulls/comments reply endpoint documentation
</function_calls>skills/receiving-code-review/SKILL.md (1)
211-217: LGTM!The GitHub PR Mechanics section correctly references the new
github-reviews.mdfile and provides clear signposting. The link structure and summary are appropriate.skills/requesting-code-review/SKILL.md (1)
105-111: LGTM!The GitHub PR Mechanics section structure and content are clear. After fixing the path reference on line 112, this section will correctly guide reviewers to the appropriate documentation.
skills/requesting-code-review/github-reviews.md (2)
8-8: Capitalize "GitHub" per style guidelines.Line 8 uses lowercase "github.com"; use "GitHub" per official naming.
Apply this diff:
-1. Repository remote hosted on github.com +1. Repository remote hosted on GitHub.comAlso applies to: 14-14
8-8: I'll help you verify the technical concerns raised in this review comment. Let me start by examining the file content and then verify the specific issues.
<function_calls>
cat -n skills/requesting-code-review/github-reviews.md
</function_calls>Now let me verify the GitHub API endpoint and gh CLI flag conventions:
<function_calls>
GitHub API endpoint POST /pulls/{number}/comments line specific review comments
</function_calls><function_calls>
gh CLI flag -f vs -F numeric parameters commit_id line
</function_calls><function_calls>
"github.com" vs "GitHub" style guide capitalization
</function_calls>
| - Posting findings (after parent approval) | ||
| - Forbidden actions | ||
|
|
||
| See template at: skills/collaboration/requesting-code-review/code-reviewer.md |
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.
Fix broken template path reference.
Line 112 references an incorrect path: skills/collaboration/requesting-code-review/code-reviewer.md. The actual template location is skills/requesting-code-review/code-reviewer.md (no collaboration/ directory level).
Apply this diff:
-See template at: skills/collaboration/requesting-code-review/code-reviewer.md
+See template at: skills/requesting-code-review/code-reviewer.mdThis path issue was flagged in the PR review and must be corrected before merge.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| See template at: skills/collaboration/requesting-code-review/code-reviewer.md | |
| See template at: skills/requesting-code-review/code-reviewer.md |
🤖 Prompt for AI Agents
In skills/requesting-code-review/SKILL.md around line 112, the template path is
incorrect — update the referenced path from
"skills/collaboration/requesting-code-review/code-reviewer.md" to the correct
path "skills/requesting-code-review/code-reviewer.md" so the link points to the
actual template location; confirm the change and run a quick link check to
ensure it resolves.
Motivation and Context
I've noticed Claude struggling to interact with GitHub PRs via the gh CLI - fetching PR content, posting comments, responding to review threads, etc. This adds focused command references for the two roles that interact with PRs:
It is broken into two separate reference files to help avoid context pollution, given many commands will not be required, depending on the workflow. I am open to different arrangements here though!
How Has This Been Tested?
I've written and reviewed against the
writing-skillsstandard in this repo. These are reference docs (command lookups), not discipline-enforcing skills, so pressure testing isn't applicable.I've tested the commands themselves and pointed this PR back at itself!
Breaking Changes
I do not believe so?
Types of changes
Checklist
Additional context
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.