Repository navigation
refactor: rewrite pr-review skill as review-only with design guidance - #3918
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the .claude “pr-review” skill to be review-only (no file modifications), adds explicit design/OO guidance for reviewers, and restructures the review report into severity-tiered tables.
Changes:
- Converts the workflow from “fix-and-report” to report-only with explicit “do not modify files” rules.
- Adds design review guidance (modularity, polymorphism opportunities, missing abstractions).
- Changes the output format to Blocking / High / Medium findings tables and updates recommendation wording.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+122
to
+130
| <<<<<<< Updated upstream | ||
| Pick exactly ONE: Ready to merge | Ready after fixing remaining issues | Needs significant rework | ||
| ||||||| Stash base | ||
| [Pick exactly ONE: "Ready to merge" | "Ready after fixing remaining issues" | "Needs significant rework"] | ||
| [1 sentence justification] | ||
| ======= | ||
| [Pick ONE: "Ready to merge" | "Ready after addressing findings" | "Needs significant rework"] | ||
| [1 sentence justification] | ||
| >>>>>>> Stashed changes |
There was a problem hiding this comment.
The ## Recommendation section still contains unresolved git merge conflict markers (<<<<<<<, |||||||, =======, >>>>>>>). This will render the skill instructions ambiguous/broken; please resolve the conflict and keep a single, consistent recommendation template (one set of options + justification).
Suggested change
| <<<<<<< Updated upstream | |
| Pick exactly ONE: Ready to merge | Ready after fixing remaining issues | Needs significant rework | |
| ||||||| Stash base | |
| [Pick exactly ONE: "Ready to merge" | "Ready after fixing remaining issues" | "Needs significant rework"] | |
| [1 sentence justification] | |
| ======= | |
| [Pick ONE: "Ready to merge" | "Ready after addressing findings" | "Needs significant rework"] | |
| [1 sentence justification] | |
| >>>>>>> Stashed changes | |
| [Pick exactly ONE: "Ready to merge" | "Ready after fixing remaining issues" | "Needs significant rework"] | |
| [1 sentence justification] |
Comment on lines
94
to
96
| - `git diff origin/main..HEAD | bob "Review this diff for correctness, | ||
| security, and design quality. Be specific about line-level issues."` | ||
|
|
There was a problem hiding this comment.
The second bob command is written as an inline-code span that wraps across a newline. Markdown inline code spans can't contain line breaks (CommonMark), so this may render incorrectly and/or be hard to copy-paste. Consider putting the command on one line, or switch to a fenced code block for multi-line commands.
Suggested change
| - `git diff origin/main..HEAD | bob "Review this diff for correctness, | |
| security, and design quality. Be specific about line-level issues."` | |
| - `git diff origin/main..HEAD | bob "Review this diff for correctness, security, and design quality. Be specific about line-level issues."` |
Conflict markers were reintroduced by rebase. Also re-fix the bob command inline code line break. Signed-off-by: Jonathan Springer <jps@s390x.com>
jonpspri
force-pushed
the
jps-pr-review-skill-rewrite
branch
from
March 30, 2026 09:54
68109e9 to
99045e0
Compare
calculus-ask
pushed a commit
to calculus-ask/mcp-context-forge
that referenced
this pull request
Mar 30, 2026
…BM#3918) Conflict markers were reintroduced by rebase. Also re-fix the bob command inline code line break. Signed-off-by: Jonathan Springer <jps@s390x.com>
msureshkumar88
pushed a commit
that referenced
this pull request
May 13, 2026
…3918) Conflict markers were reintroduced by rebase. Also re-fix the bob command inline code line break. Signed-off-by: Jonathan Springer <jps@s390x.com>
kamath-a
pushed a commit
to kamath-a/mcp-context-forge
that referenced
this pull request
Jun 26, 2026
…BM#3918) Conflict markers were reintroduced by rebase. Also re-fix the bob command inline code line break. Signed-off-by: Jonathan Springer <jps@s390x.com>
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
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.
Summary
Note: There are unresolved merge conflict markers in the Recommendation section that need to be cleaned up before merge.