diff --git a/skills/receiving-code-review/SKILL.md b/skills/receiving-code-review/SKILL.md index 85d8b036..f6bf527e 100644 --- a/skills/receiving-code-review/SKILL.md +++ b/skills/receiving-code-review/SKILL.md @@ -207,3 +207,11 @@ You understand 1,2,3,6. Unclear on 4,5. Verify. Question. Then implement. No performative agreement. Technical rigor always. + +## GitHub PR Mechanics + +If responding to feedback on a GitHub PR, see `github-reviews.md` for: +- Reading review comments (general and line-specific) +- Responding to feedback (comment vs thread reply) +- Commit strategy and requesting re-review +- Forbidden actions diff --git a/skills/receiving-code-review/github-reviews.md b/skills/receiving-code-review/github-reviews.md new file mode 100644 index 00000000..7dd45aef --- /dev/null +++ b/skills/receiving-code-review/github-reviews.md @@ -0,0 +1,138 @@ +# GitHub Review Mechanics (For Implementers) + +Command reference for agents responding to PR feedback on GitHub. + +## Prerequisites + +**Required:** +1. Repository remote hosted on github.com +2. GitHub CLI (`gh`) installed and authenticated + +**Verify before first use (skip if already confirmed):** +```bash +# Check remote is GitHub +git remote get-url origin # Should contain "github.com" + +# Check gh CLI available +gh --version +``` + +**If checks fail:** Use alternative workflow or ask your human partner for guidance. + +## Reading Review Feedback + +### Basic View +```bash +# Overview with comments +gh pr view + +# View what changed +gh pr diff + +# Check CI status +gh pr checks +``` + +### Structured Data +```bash +# Get comments, reviews, and pending reviewers +gh pr view --json comments,reviews,reviewRequests + +# Get line-specific review comments with full context +gh api repos/{owner}/{repo}/pulls/{number}/comments + +# Extract key fields for processing +gh api repos/{owner}/{repo}/pulls/{number}/comments \ + --jq '.[] | {id, path, start_line, line, body}' +``` + +**Multi-line comments:** `start_line` shows where comment begins, `line` shows where it ends. If `start_line` is null, it's a single-line comment. + +## Responding to Feedback + +### General PR Comment +```bash +gh pr comment --body "Addressed review feedback: +- Fixed validation logic per comment on utils.py:42 +- Refactored error handling per comment on api.py:15 +All changes pushed in commit abc123" +``` +**Use for:** Summarizing multiple fixes, providing overall context + +### Reply to Specific Thread +```bash +# First, get comment IDs +gh api repos/{owner}/{repo}/pulls/{number}/comments \ + --jq '.[] | {id, path, line, body}' + +# Reply to specific comment thread +gh api repos/{owner}/{repo}/pulls/comments/{comment_id}/replies \ + -f body="Fixed in commit abc123. [Technical explanation]" +``` +**Use for:** +- Answering technical questions +- Providing file/line-specific context +- Pushing back on suggestions with reasoning +- Threading discussion to maintain context + +### Editing Your Own Comments +```bash +gh api repos/{owner}/{repo}/pulls/comments/{comment_id} \ + -X PATCH \ + -f body="Corrected: [updated text]" +``` +**Use for:** Fixing errors in your own posts (not others') + +## Commit Strategy + +```bash +# One commit per distinct fix (preferred) +git commit -m "Fix: [specific issue from review]" + +# Push changes (triggers notification to reviewers) +git push +``` + +**Commit messages should:** +- Reference what review feedback was addressed +- Be specific, not just "address review comments" + +**When to push:** +- After fixing all related items +- After all items if changes are tightly coupled +- Incrementally if fixes are independent + +## Requesting Re-Review + +```bash +# 1. Push your changes first +git push + +# 2. Verify CI passes before requesting re-review +gh pr checks + +# 3. Request re-review (only after CI is green) +gh pr edit --add-reviewer @username + +# Or request from a team +gh pr edit --add-reviewer @org/team +``` + +**When to request:** +- After implementing ALL feedback items +- After pushing changes +- After CI checks pass + +**Note:** Pushing commits notifies reviewers automatically. Only use explicit re-review request when you need formal re-review. + +## Forbidden Actions + +**NEVER do any of the following:** + +| Action | Why Forbidden | +|--------|---------------| +| Mark comments as resolved | Reviewer's responsibility, not yours | +| `gh pr close` | Closing PRs is human decision | +| `gh pr merge` | Merging is human decision | +| `gh pr review --approve` | Cannot approve your own work | +| `git push --force` | Destructive to PR history | diff --git a/skills/requesting-code-review/SKILL.md b/skills/requesting-code-review/SKILL.md index f8b1a565..074fb51b 100644 --- a/skills/requesting-code-review/SKILL.md +++ b/skills/requesting-code-review/SKILL.md @@ -102,4 +102,11 @@ You: [Fix progress indicators] - Show code/tests that prove it works - Request clarification -See template at: requesting-code-review/code-reviewer.md +## GitHub PR Mechanics + +If reviewing a PR hosted on GitHub, see `github-reviews.md` for: +- Fetching PR content and existing discussion +- Posting findings (after parent approval) +- Forbidden actions + +See template at: skills/collaboration/requesting-code-review/code-reviewer.md diff --git a/skills/requesting-code-review/github-reviews.md b/skills/requesting-code-review/github-reviews.md new file mode 100644 index 00000000..500fa484 --- /dev/null +++ b/skills/requesting-code-review/github-reviews.md @@ -0,0 +1,103 @@ +# GitHub Review Mechanics (For Reviewers) + +Command reference for code-reviewer agents analyzing PRs hosted on GitHub. + +## Prerequisites + +**Required:** +1. Repository remote hosted on github.com +2. GitHub CLI (`gh`) installed and authenticated + +**Verify before first use (skip if already confirmed):** +```bash +# Check remote is GitHub +git remote get-url origin # Should contain "github.com" + +# Check gh CLI available +gh --version +``` + +**If checks fail:** Use alternative workflow or ask your human partner for guidance. + +## Fetching PR Content + +### Basic PR Information +```bash +# Overview: title, body, state, author +gh pr view + +# View the actual code changes +gh pr diff + +# Check CI/build status +gh pr checks +``` + +### Existing Discussion +```bash +# Get comments and reviews as JSON +gh pr view --json comments,reviews,reviewRequests + +# Get line-specific review comments with file and line info +gh api repos/{owner}/{repo}/pulls/{number}/comments + +# Extract specific fields for processing +gh api repos/{owner}/{repo}/pulls/{number}/comments \ + --jq '.[] | {id, path, start_line, line, body}' +``` + +**Multi-line comments:** `start_line` shows where comment begins, `line` shows where it ends. If `start_line` is null, it's a single-line comment. + +## Posting Review Findings + +**IMPORTANT:** Only post to GitHub AFTER: +1. Reporting complete findings to parent agent +2. Receiving explicit permission to post + +### General PR Comment +```bash +gh pr comment --body "Review summary: +- Finding 1 +- Finding 2 +..." +``` +**Use for:** Overall summary, non-code-specific observations + +### Formal Review +```bash +# Comment only (no approval/rejection) +gh pr review --comment --body "Reviewed changes. See inline comments for details." +``` +**Use for:** Structured review submission + +### Line-Specific Comments +```bash +# Post comment on specific line +gh api repos/{owner}/{repo}/pulls/{number}/comments \ + -f body="Issue: [description]" \ + -f path="src/file.ts" \ + -f commit_id="$(gh pr view --json headRefOid -q .headRefOid)" \ + -F line=42 + +# Post comment spanning multiple lines +gh api repos/{owner}/{repo}/pulls/{number}/comments \ + -f body="Issue: [description]" \ + -f path="src/file.ts" \ + -f commit_id="$(gh pr view --json headRefOid -q .headRefOid)" \ + -F start_line=40 \ + -F line=45 +``` +**Use for:** Inline feedback on specific code locations + +## Forbidden Actions + +**NEVER do any of the following:** + +| Action | Why Forbidden | +|--------|---------------| +| `gh pr close` | Closing PRs is human decision | +| `gh pr merge` | Merging is human decision | +| `gh pr review --approve` | Approval requires human judgment | +| `gh pr review --request-changes` | Blocking PRs requires human judgment | +| `git push --force` | Destructive to PR history | +| Post without permission | Parent agent must approve first |