-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[PR-Reviewer] Standardize PR review output format with collapsible sections #32784
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,28 +2,129 @@ | |||||
|
|
||||||
| ## Review Output Format | ||||||
|
|
||||||
| Structure your review in this order: | ||||||
| **CRITICAL**: All reviews MUST be saved to a file named `Review_Feedback_Issue_XXXXX.md` (replace XXXXX with the actual issue number). | ||||||
|
|
||||||
| Structure your review in this exact format: | ||||||
|
|
||||||
| ```markdown | ||||||
| # Review Feedback: PR #XXXXX - [PR Title] | ||||||
|
|
||||||
| ## Recommendation | ||||||
| β **Approve** - Ready to merge | ||||||
| β οΈ **Request Changes** - Issues must be fixed | ||||||
| π¬ **Comment** - Feedback but not blocking | ||||||
| βΈοΈ **Paused** - Cannot complete review (conflicts, environment issues, etc.) | ||||||
|
|
||||||
| **Required changes** (if any): | ||||||
| 1. [First required change] | ||||||
| 2. [Second required change] | ||||||
|
|
||||||
| **Recommended changes** (if any): | ||||||
| 1. [First suggested improvement] | ||||||
| 2. [Second suggested improvement] | ||||||
|
|
||||||
| --- | ||||||
|
|
||||||
| <details> | ||||||
| <summary><b>π For full PR Review from agent, expand here</b></summary> | ||||||
|
|
||||||
| ## Summary | ||||||
| [2-3 sentence overview of what the PR does and your assessment] | ||||||
|
|
||||||
| --- | ||||||
|
|
||||||
| ## Code Review | ||||||
| [Your analysis of the code changes - see core-guidelines.md for details] | ||||||
|
|
||||||
| --- | ||||||
|
|
||||||
| ## Test Coverage Review | ||||||
| [Analysis of tests added/modified in the PR] | ||||||
|
|
||||||
| ### Issues Found in Tests (if any) | ||||||
| [Specific test issues with file locations and line numbers] | ||||||
|
|
||||||
| --- | ||||||
|
|
||||||
| ## Testing | ||||||
| [Results from your manual testing with the Sandbox app] | ||||||
|
|
||||||
| ## Issues Found | ||||||
| [Any problems, concerns, or questions - or "None" if everything looks good] | ||||||
| ### Manual Testing (if applicable) | ||||||
| [Your testing results] | ||||||
|
|
||||||
| ## Recommendation | ||||||
| β **Approve** - Ready to merge | ||||||
| β οΈ **Request Changes** - Issues must be fixed | ||||||
| π¬ **Comment** - Feedback but not blocking | ||||||
| βΈοΈ **Paused** - Cannot complete review (conflicts, environment issues, etc.) | ||||||
| ### Recommended Testing Steps (if checkpoint created) | ||||||
| [Commands for others to test] | ||||||
|
|
||||||
| --- | ||||||
|
|
||||||
| ## Security Review | ||||||
| [Security assessment - or "β No security concerns" if none found] | ||||||
|
|
||||||
| --- | ||||||
|
|
||||||
| ## Breaking Changes | ||||||
| [Breaking change analysis - or "β No breaking changes" if none found] | ||||||
|
|
||||||
| --- | ||||||
|
|
||||||
| ## Documentation | ||||||
| [Documentation review - or "β Adequate" if satisfactory] | ||||||
|
|
||||||
| --- | ||||||
|
|
||||||
| ## Issues to Address | ||||||
|
|
||||||
| ### Must Fix Before Merge | ||||||
| [Critical issues that block approval] | ||||||
|
|
||||||
| ### Should Fix (Recommended) | ||||||
| [Important improvements that should be made] | ||||||
|
|
||||||
| ### Optional Improvements | ||||||
| [Nice-to-have suggestions] | ||||||
|
|
||||||
| --- | ||||||
|
|
||||||
| ## Approval Checklist | ||||||
| - [ ] Code solves the stated problem correctly | ||||||
| - [ ] Minimal, focused changes | ||||||
| - [ ] No breaking changes | ||||||
| - [ ] Appropriate test coverage exists | ||||||
| - [ ] No security concerns | ||||||
| - [ ] Follows .NET MAUI conventions | ||||||
| [Add specific items relevant to this PR] | ||||||
|
||||||
| [Add specific items relevant to this PR] | |
| <!-- Add any PR-specific checklist items here --> |
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.
Minor inconsistency in section header formatting. In the example provided in the PR description, the recommendation in the visible section uses bold text without a section header (
**Recommendation**: β Approve), but the template at line 12 shows it as a proper markdown section header (## Recommendation).Consider updating line 10-16 to show the expected output format more clearly:
This would match the example in the PR description and be more explicit about the expected format.