Browse Source

Feat: Improve PR Reviewer Rules (#4651)

Daniel 8 months ago
parent
commit
87186067f9

+ 29 - 6
.roo/rules-pr-reviewer/1_workflow.xml

@@ -26,6 +26,28 @@
   </step>
 
   <step number="2">
+    <name>Fetch Associated Issue (If Any)</name>
+    <instructions>
+      Check the pull request body for a reference to a GitHub issue (e.g., "Fixes #123", "Closes #456").
+      If an issue is referenced, use the GitHub MCP tool to fetch its details:
+
+      <use_mcp_tool>
+      <server_name>github</server_name>
+      <tool_name>get_issue</tool_name>
+      <arguments>
+      {
+        "owner": "[owner]",
+        "repo": "[repo]",
+        "issue_number": [issue_number]
+      }
+      </arguments>
+      </use_mcp_tool>
+
+      The issue description and comments can provide valuable context for the review.
+    </instructions>
+  </step>
+
+  <step number="3">
     <name>Fetch Pull Request Diff</name>
     <instructions>
       Get the pull request diff to understand the changes:
@@ -44,7 +66,7 @@
     </instructions>
   </step>
 
-  <step number="3">
+  <step number="4">
     <name>Check Out Pull Request Locally</name>
     <instructions>
       Use the GitHub CLI (e.g. `gh pr checkout <PR_NUMBER>`) to check out the pull request locally after fetching
@@ -61,7 +83,7 @@
     </instructions>
   </step>
 
-  <step number="4">
+  <step number="5">
     <name>Fetch Existing PR Comments</name>
     <instructions>
       Get existing comments to understand the current discussion state:
@@ -82,10 +104,11 @@
     </instructions>
   </step>
 
-  <step number="5">
+  <step number="6">
     <name>Perform Comprehensive Review</name>
     <instructions>
       Review the pull request thoroughly:
+      - Verify that the changes are directly related to the linked issue and do not include unrelated modifications.
       - Focus primarily on the changes made in the PR.
       - Prioritize code quality, code smell, structural consistency, and for UI-related changes, ensure proper internationalization (i18n) is applied.
       - Watch for signs of technical debt (e.g., overly complex logic, lack of abstraction, tight coupling, missing tests, TODOs).
@@ -106,7 +129,7 @@
     </instructions>
   </step>
 
-  <step number="6">
+  <step number="7">
     <name>Prepare Review Comments</name>
     <instructions>
       Format your review comments following these guidelines:
@@ -128,7 +151,7 @@
     </instructions>
   </step>
 
-  <step number="7">
+  <step number="8">
     <name>Preview Review with User</name>
     <instructions>
       Always show the user a preview of your review suggestions and comments before taking any action.
@@ -154,7 +177,7 @@
     </instructions>
   </step>
 
-  <step number="8">
+  <step number="9">
     <name>Submit Review</name>
     <instructions>
       Based on user preference, submit the review:

+ 2 - 0
.roo/rules-pr-reviewer/2_best_practices.xml

@@ -1,8 +1,10 @@
 <best_practices>
   - Always fetch and review the entire PR diff before commenting
+  - Check for and review any associated issue for context
   - Check out the PR locally for better context understanding
   - Review existing comments to avoid duplicate feedback
   - Focus on the changes made, not unrelated code
+  - Ensure all changes are directly related to the linked issue
   - Use a friendly, curious tone in all comments
   - Ask questions rather than making assumptions
   - Provide actionable feedback with specific suggestions

+ 2 - 0
.roo/rules-pr-reviewer/3_common_mistakes_to_avoid.xml

@@ -2,9 +2,11 @@
   - Running tests or executing code during review
   - Making judgmental or harsh comments
   - Providing feedback on code outside the PR's scope
+  - Overlooking unrelated changes not tied to the main issue
   - Using excessive praise or unnecessary formatting
   - Submitting comments without user preview/approval
   - Ignoring existing PR comments and discussions
+  - Forgetting to check for an associated issue for additional context
   - Missing critical security or performance issues
   - Not checking for proper i18n in UI changes
   - Failing to suggest breaking up large PRs