|
| 1 | +# Copilot Code Review Guidelines |
| 2 | + |
| 3 | +--- |
| 4 | +This file configures GitHub Copilot Code Review according to GitHub’s official guidance: [Use Copilot Code Review](https://docs.github.com/en/copilot/how-tos/use-copilot-agents/request-a-code-review/use-code-review). |
| 5 | + |
| 6 | +## Project Context |
| 7 | + |
| 8 | +**Language:** Ruby ~> 4 |
| 9 | +**Framework:** [trailblazer](https://trailblazer.to/) |
| 10 | +**Testing:** [minitest](https://docs.seattlerb.org/minitest/) following [BetterSpecs](https://evenbetterspecs.github.io/) guidelines |
| 11 | + |
| 12 | +--- |
| 13 | + |
| 14 | +## Purpose & Scope |
| 15 | + |
| 16 | +Focused inline review guidance for Ruby pull requests—style, tests, performance, and tone. (CI setup, contributor workflows, and documentation policies belong in separate docs.) |
| 17 | + |
| 18 | +--- |
| 19 | + |
| 20 | +## 1. Style Guides |
| 21 | + |
| 22 | +> ⚠️ **Be constructive:** Frame style feedback as suggestions (e.g., “Consider using…”). |
| 23 | +
|
| 24 | +All standard rules follow the [Shopify Ruby Style Guide](https://ruby-style-guide.shopify.dev/); only team-specific overrides are listed below: |
| 25 | +- Indentation and line length enforced by RuboCop per project config. |
| 26 | +- Use guard clauses, and explicit receivers. |
| 27 | +- Prefer Railway Oriented Programming (ROP) for error handling and flow. |
| 28 | +- Leverage dry-rb conventions and Trailblazer patterns for clarity and maintainability. |
| 29 | +- Keep methods short. 1 line is fine. 5 is a lot. At 7, you should consider extracting a method. |
| 30 | + |
| 31 | +--- |
| 32 | + |
| 33 | +## 2. Tone and Delivery |
| 34 | + |
| 35 | +> 🤝 **Be succinct:** Don't overwhelm with too many minor issues; prioritize key improvements. |
| 36 | +
|
| 37 | +- Use phrasing like “Consider extracting…” or “You might simplify…” rather than absolutes. |
| 38 | +- Keep comments concise—2–3 sentences max. |
| 39 | + |
| 40 | +--- |
| 41 | + |
| 42 | +## 3. Conventions & Readability |
| 43 | + |
| 44 | +> ✨ **Keep it clear:** Propose naming and structure that clarify intent. |
| 45 | +
|
| 46 | +- **SOLID principles:** Single responsibility; inject dependencies via initializers. |
| 47 | +- **Descriptive names:** Recommend clear class, method, and variable names. |
| 48 | +- **DRY:** Suggest extracting duplicate logic into modules or service objects. |
| 49 | +- **Method size:** Flag methods >= 8 lines; suggest splitting into helpers. |
| 50 | +- Ensure files have EOF newline. |
| 51 | +- Flag trailing white space in code and comments. |
| 52 | +- Always ensure rubocop runs cleanly. |
| 53 | + |
| 54 | +--- |
| 55 | + |
| 56 | +## 4. Testing and Coverage |
| 57 | + |
| 58 | +> 🧪 **Be thorough:** Highlight gaps in meaningful test coverage. |
| 59 | +
|
| 60 | +- Flag missing or outdated specs for changed behaviors (unit, integration, request). |
| 61 | +- Suggest tests for edge cases: error conditions, invalid inputs, boundary values, and potential `nil` handling or pointer exceptions. Follow best practices from the [BetterSpecs style guide](https://evenbetterspecs.github.io/). |
| 62 | +- Encourage descriptive `context` blocks and example names per [BetterSpecs](https://evenbetterspecs.github.io/). |
| 63 | +- Flag missing specs when logic is updated but no tests were added or modified |
| 64 | +--- |
| 65 | + |
| 66 | +## 5. Performance and Security |
| 67 | + |
| 68 | +> 🚀 **Guard quality:** Call out patterns that may hurt performance or security. |
| 69 | +
|
| 70 | +- Detect potential N+1 queries; suggest `includes` or batching. |
| 71 | +- Attempt to identify O(n^2) or O(n log n) algorithms; recommend more efficient alternatives. |
| 72 | +- Flag raw SQL, unsanitized interpolation, or other patterns that could introduce potential SQL injection vulnerabilities; recommend parameter binding and Sequel-based alternatives. Build these into a model, avoid raw sequel unless absolutely necessary. |
| 73 | +- Identify unescaped user input in route methods |
| 74 | + |
| 75 | +--- |
| 76 | + |
| 77 | +## 6. Commit Message and PR title guidelines |
| 78 | + |
| 79 | +> 📝 **Be clear:** Ensure commit messages and PR titles reflect changes accurately. |
| 80 | +
|
| 81 | +- We use Conventional Commits. Ensure commit messages follow the format: `<type>(<scope>): <description>`. |
| 82 | +- We use Semantic PR titles as well, ensure PR titles also match the conventional commit format. |
| 83 | + |
| 84 | +## 7. Balancing Feedback |
| 85 | + |
| 86 | +> ⚖️ **Be selective:** Focus on the highest-impact issues. |
| 87 | +
|
| 88 | +- Limit critical suggestions to the top 3–5 items per review. |
| 89 | +- Combine minor style tweaks into a single comment when possible. |
| 90 | + |
| 91 | +--- |
| 92 | + |
| 93 | +## 8. Quick Reference |
| 94 | + |
| 95 | +- **Max comment length:** 2–3 sentences. |
| 96 | +- **Severity tags:** `[Major]`, `[Medium]`, `[Minor]`. |
| 97 | +- **Mandatory header:** `Enable frozen-string-literal`. |
| 98 | +- **Top issues:** 3–5 critical comments. |
| 99 | + |
| 100 | +--- |
0 commit comments