|
| 1 | +# GitHub Copilot Code Review Instructions |
| 2 | + |
| 3 | +## Review Philosophy |
| 4 | + |
| 5 | +- Only comment when you have HIGH CONFIDENCE (>80%) that an issue exists |
| 6 | +- Be concise: one sentence per comment when possible |
| 7 | +- Focus on actionable feedback, not observations |
| 8 | +- When reviewing text, only comment on clarity issues if the text is genuinely confusing or could lead to errors. |
| 9 | + "Could be clearer" is not the same as "is confusing" - stay silent unless HIGH confidence it will cause problems |
| 10 | + |
| 11 | +## Priority Areas (Review These) |
| 12 | + |
| 13 | +### Security & Safety |
| 14 | + |
| 15 | +- Unsafe code blocks without justification |
| 16 | +- Command injection risks (shell commands, user input) |
| 17 | +- Path traversal vulnerabilities |
| 18 | +- Credential exposure or hardcoded secrets |
| 19 | +- Missing input validation on external data |
| 20 | +- Improper error handling that could leak sensitive info |
| 21 | + |
| 22 | +### Correctness Issues |
| 23 | + |
| 24 | +- Logic errors that could cause panics or incorrect behavior |
| 25 | +- Resource leaks (files, connections, memory) |
| 26 | +- Off-by-one errors or boundary conditions |
| 27 | +- Optional types that don't need to be optional |
| 28 | +- Booleans that should default to false but are set as optional |
| 29 | +- Overly defensive code that adds unnecessary checks |
| 30 | +- Unnecessary comments that just restate what the code already shows (remove them) |
| 31 | + |
| 32 | +### Architecture & Patterns |
| 33 | + |
| 34 | +- Code that violates existing patterns in the codebase |
| 35 | +- Missing error handling |
| 36 | +- Code that is not following [Effective Go](https://go.dev/doc/effective_go) best practices |
| 37 | +- Violating [Kubernetes API guidelines](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api_changes.md) |
| 38 | + |
| 39 | +## Project-Specific Context |
| 40 | + |
| 41 | +- See [AGENTS.md](../AGENTS.md) in the root directory for project guidelines and architecture decisions. |
| 42 | + |
| 43 | +## CI Pipeline Context |
| 44 | + |
| 45 | +**Important**: You review PRs immediately, before CI completes. Do not flag issues that CI will catch. |
| 46 | + |
| 47 | +### What Our CI Checks |
| 48 | + |
| 49 | +- `.github/workflows/test-go.yaml` - Code generation, linting, and tests for Go source code |
| 50 | +- `.github/workflows/test-python.yaml` - unit and integration tests for Python source code |
| 51 | +- `.github/workflows/test-rust.yaml` - unit and integration tests for Rust source code |
| 52 | +- `.github/workflows/test-e2e.yaml` - e2e tests |
| 53 | +- `.github/workflows/build-and-push-images.yaml` - build and push container images |
| 54 | + |
| 55 | +## Skip These (IMPORTANT) |
| 56 | + |
| 57 | +Do not comment on: |
| 58 | + |
| 59 | +- **Auto generated code** - CI handles this (make generate) |
| 60 | +- **Style/formatting** - CI handles this (gofmt, black, prettier) |
| 61 | +- **Test failures** - CI handles this (full test suite) |
| 62 | +- **Missing dependencies** - CI handles this |
| 63 | + |
| 64 | +## Response Format |
| 65 | + |
| 66 | +When you identify an issue: |
| 67 | + |
| 68 | +1. **State the problem** (1 sentence) |
| 69 | +2. **Why it matters** (1 sentence, only if not obvious) |
| 70 | +3. **Suggested fix** (code snippet or specific action) |
| 71 | + |
| 72 | +## When to Stay Silent |
| 73 | + |
| 74 | +If you're uncertain whether something is an issue, don't comment. False positives create noise and reduce trust in the review process. |
0 commit comments