Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion .claude/commands/review-cycle.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,39 @@ Please execute the following steps:
2. Check for unresolved threads:
!go tool gh-helper reviews fetch --unresolved-only

3. If there are unresolved threads, please help me address the feedback and then use `/project:review-respond` to reply to all threads.
3. If there are no unresolved threads, report that the review cycle is clean and stop here.

4. If there are unresolved threads, address each one:

For each unresolved thread, evaluate the feedback and choose a response strategy:

- **Code fix needed**: Make the fix in code, then continue to step 5.
- **Explanation only** (no code change needed): Reply with reasoning why current code is correct, resolve, and move to the next thread.
- **Praise/positive comment**: Acknowledge briefly (e.g., "Thank you!") and resolve.

**Reply content guidelines — always write a meaningful reply:**
- Do NOT just post a commit hash. Explain what was changed and why.
- For code fixes: Describe the specific change made to address the feedback (e.g., "Removed the redundant nil check — `ListVariables()` calls `ensureRegistry()` internally, so the explicit guard was unnecessary and could prevent first-use initialization.")
- For explanations: Provide concrete reasoning, not just "this is intentional."
- Keep it concise but substantive: 1-3 sentences is ideal.

Thread reply examples:
```bash
# Code fix — explain what was changed
go tool gh-helper threads reply THREAD_ID --commit-hash abc123 --resolve \
--message "Removed the redundant nil check. ListVariables() calls ensureRegistry() internally, so the explicit guard was preventing first-use initialization."

# Explanation only — provide reasoning
go tool gh-helper threads reply THREAD_ID --resolve \
--message "This is intentional: the regex requires \\s+ after SET to avoid matching bare SET as a variable context, which would conflict with other SET usages."

# Acknowledge praise
go tool gh-helper threads reply THREAD_ID --message "Thank you!" --resolve
```

5. After addressing all threads with code changes, commit and push the fixes.

6. With the new commit hash, reply to and resolve all code-fix threads, then request a new review to re-validate:
!go tool gh-helper reviews wait --request-review

7. Repeat from step 2 until there are no unresolved threads.
31 changes: 17 additions & 14 deletions .claude/commands/review-respond.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
name: Review Respond
description: Reply to all review threads with commit hash and resolve
arguments: "[commit-message]"
arguments: "[commit_message]"
---

# Respond to Review Threads
Expand All @@ -17,38 +17,41 @@ After addressing review feedback, please:
2. Find all unresolved threads (including outdated ones) and respond to each one:
!go tool gh-helper reviews fetch --unresolved-only

For each thread ID found above, reply with the CORRECT commit hash where that specific issue was fixed and resolve it, regardless of whether it's marked as outdated.
For each thread ID found above, reply and resolve it, regardless of whether it's marked as outdated.

**Reply content guidelines — always write a meaningful reply:**
- Do NOT just post a commit hash. Explain what was changed and why.
- For code fixes: Describe the specific change made to address the feedback.
- For explanations: Provide concrete reasoning, not just "this is intentional."
- Keep it concise but substantive: 1-3 sentences is ideal.

**Response strategy per thread type:**

- **Code fix needed**: Make the fix, commit, then reply with commit hash and resolve
- **Code fix needed**: Make the fix, commit, then reply with commit hash and explanation, and resolve
- **Explanation only** (no code change needed): Reply with reasoning why current code is correct, then resolve
- **Praise/positive comment**: Acknowledge briefly (e.g., "Thank you!") and resolve — don't leave these unresolved

When replying to threads, include a brief explanation of the fix:
- Use `--message "Brief explanation of what was fixed"` for single-line responses
- For multi-line responses, use stdin with heredoc
- Default format: "Fixed in [commit-hash] - [brief explanation]$ARGUMENTS"

Examples:
```bash
# Single-line response
go tool gh-helper threads reply THREAD_ID --commit-hash abc123 --message "Fixed by making CLI_SKIP_SYSTEM_COMMAND read-only" --resolve
# Code fix — explain what was changed
go tool gh-helper threads reply THREAD_ID --commit-hash abc123 --resolve \
--message "Removed the redundant nil check. ListVariables() calls ensureRegistry() internally, so the explicit guard was preventing first-use initialization."

# Multi-line response for complex fixes
cat <<EOF | go tool gh-helper threads reply THREAD_ID --commit-hash abc123 --resolve
Fixed by switching from buffering to streaming output.
This prevents DoS attacks from commands with large output.
Switched from buffering to streaming output.
This prevents memory issues from commands with large output.
EOF

# Acknowledge praise comment (no code change)
go tool gh-helper threads reply THREAD_ID --message "Thank you!" --resolve

# Explanation-only response (no code change)
go tool gh-helper threads reply THREAD_ID --message "This is intentional because ..." --resolve
go tool gh-helper threads reply THREAD_ID --resolve \
--message "This is intentional: the regex requires \\s+ after SET to avoid matching bare SET as a variable context."
```

Note: Even threads marked as "outdated" should be replied to and resolved, as they may contain valuable feedback that was addressed.

3. After all threads are resolved, request a new review:
!go tool gh-helper reviews wait --request-review
!go tool gh-helper reviews wait --request-review
6 changes: 5 additions & 1 deletion .gemini/styleguide.md
Original file line number Diff line number Diff line change
Expand Up @@ -423,4 +423,8 @@ When reviewing pull requests, please focus on:
4. **Resource Management**: Are resources (files, connections, etc.) properly closed?
5. **Documentation**: Are public APIs and complex logic adequately documented?

Please avoid suggesting changes that are purely stylistic unless they significantly impact readability or maintainability.
Please avoid suggesting changes that are purely stylistic unless they significantly impact readability or maintainability.

### Avoid Praise-Only Comments

**DO NOT** create review threads that only praise existing code without actionable feedback. Comments like "This is a good improvement" or "The test coverage is excellent" create noise that must be manually resolved by the author. If code is good, simply don't comment on it. Every review comment should contain actionable feedback or a specific concern.