mirror of
https://github.com/freeCodeCamp/freeCodeCamp.git
synced 2026-05-28 18:26:54 +00:00
fix(copilot): eliminate verbose review summaries and restrict to staff (#65587)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: raisedadead <1884376+raisedadead@users.noreply.github.com>
This commit is contained in:
@@ -1,8 +1,57 @@
|
|||||||
# GitHub Copilot Code Review Instructions
|
# GitHub Copilot Code Review Instructions
|
||||||
|
|
||||||
## Focus Areas
|
## Core Principles
|
||||||
|
|
||||||
This document guides Copilot code review on two key areas:
|
**CHECK REQUESTER AUTHORIZATION FIRST:**
|
||||||
|
|
||||||
|
Before providing any review, verify if the person who requested the Copilot review is a member of the `@freecodecamp/staff` team.
|
||||||
|
|
||||||
|
- If the requester is NOT part of `@freecodecamp/staff`, respond with ONLY this message:
|
||||||
|
```
|
||||||
|
Please use AI reviews in your local environment, and leave this feature to be used by fCC mods and Staff.
|
||||||
|
```
|
||||||
|
Do not provide any other feedback or review comments.
|
||||||
|
|
||||||
|
- If the requester IS part of `@freecodecamp/staff`, proceed with the review following the guidelines below.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
**BE EXTREMELY MINIMAL.** Only provide actionable feedback.
|
||||||
|
|
||||||
|
**Skip all non-essential content:**
|
||||||
|
- Do not generate "Pull request overview" sections
|
||||||
|
- Do not create "Changes:" lists describing what was changed
|
||||||
|
- Do not create "Reviewed changes" sections or tables
|
||||||
|
- Do not list files with change descriptions
|
||||||
|
- Do not count how many files were reviewed
|
||||||
|
- Do not summarize what the PR does (visible in the diff)
|
||||||
|
|
||||||
|
**Focus only on problems:**
|
||||||
|
- Comment ONLY on actual issues that need fixing
|
||||||
|
- Keep each comment 1-3 sentences
|
||||||
|
- If everything is correct, provide no output
|
||||||
|
|
||||||
|
**Bad example (from PR 65578):**
|
||||||
|
```
|
||||||
|
## Pull request overview
|
||||||
|
This PR addresses issue 65331 by replacing em dash characters...
|
||||||
|
|
||||||
|
### Reviewed changes
|
||||||
|
Copilot reviewed 4 out of 4 changed files...
|
||||||
|
|
||||||
|
| File | Description |
|
||||||
|
| ---- | ----------- |
|
||||||
|
| file1.md | Updated em dash in seed content |
|
||||||
|
```
|
||||||
|
|
||||||
|
**Good example (actionable feedback only):**
|
||||||
|
```
|
||||||
|
Line 46: Test will fail - `innerText` returns rendered `—`, not `—`.
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Focus Areas
|
||||||
|
|
||||||
1. **Test Coverage** - Ensuring adequate testing for code changes
|
1. **Test Coverage** - Ensuring adequate testing for code changes
|
||||||
2. **Pull Request Guidelines** - Compliance with contribution standards
|
2. **Pull Request Guidelines** - Compliance with contribution standards
|
||||||
@@ -40,20 +89,15 @@ Do not comment if:
|
|||||||
|
|
||||||
### Comment Style
|
### Comment Style
|
||||||
|
|
||||||
If tests are missing, provide a brief, actionable nudge:
|
If tests are missing, provide ONE brief comment:
|
||||||
|
|
||||||
```
|
```
|
||||||
Consider adding tests for the new/modified functionality in [specific file].
|
Missing tests for [specific file]. Consider:
|
||||||
For example:
|
- [specific scenario]
|
||||||
- Test case for [specific scenario]
|
- [edge case]
|
||||||
- Edge case handling for [specific condition]
|
|
||||||
```
|
```
|
||||||
|
|
||||||
If test coverage is sufficient:
|
If test coverage is sufficient, **DO NOT COMMENT**. No "LGTM" needed.
|
||||||
|
|
||||||
```
|
|
||||||
LGTM
|
|
||||||
```
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
@@ -117,39 +161,40 @@ Flag if:
|
|||||||
|
|
||||||
### Comment Style
|
### Comment Style
|
||||||
|
|
||||||
Keep feedback specific and reference the contribution guide:
|
Keep feedback minimal (one line when possible):
|
||||||
|
|
||||||
```
|
```
|
||||||
Please update your PR title to follow conventional commits format: `<type>(scope): description`
|
Update PR title to format: `<type>(scope): description`
|
||||||
See: https://contribute.freecodecamp.org/how-to-open-a-pull-request
|
See: https://contribute.freecodecamp.org/how-to-open-a-pull-request
|
||||||
```
|
```
|
||||||
|
|
||||||
```
|
```
|
||||||
Please link the related issue using `Closes #XXXXX` format in the PR description.
|
Link issue using: `Closes #XXXXX`
|
||||||
```
|
```
|
||||||
|
|
||||||
If PR guidelines are followed:
|
If PR guidelines are followed, **DO NOT COMMENT**. No "LGTM" needed.
|
||||||
|
|
||||||
```
|
|
||||||
LGTM
|
|
||||||
```
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## General Guidelines
|
## General Guidelines
|
||||||
|
|
||||||
### Avoid
|
**Focus on Actionable Issues Only**
|
||||||
|
|
||||||
- Long explanations
|
### Strict Rules
|
||||||
- Generic comments without specific pointers
|
|
||||||
- Comments on non-functional changes
|
- NO summaries, overviews, or descriptions of changes
|
||||||
- Repeating information already in the PR discussion
|
- NO tables or file listings
|
||||||
- Commenting on the same issue multiple times
|
- NO "LGTM" or affirmative comments when everything is fine
|
||||||
|
- Only comment when action is required
|
||||||
|
- Keep each comment brief and actionable
|
||||||
|
|
||||||
### Prioritization
|
### Prioritization
|
||||||
|
|
||||||
When multiple issues exist, prioritize feedback in this order:
|
When multiple issues exist, address them in order of severity:
|
||||||
|
|
||||||
1. PR title/description compliance (most visible, easiest to fix)
|
1. Security vulnerabilities or critical bugs
|
||||||
2. Missing test coverage for new functionality
|
2. Missing test coverage for new functionality
|
||||||
3. Outdated tests for modified functionality
|
3. Outdated tests for modified functionality
|
||||||
|
4. PR title/description compliance
|
||||||
|
|
||||||
|
Comment on all legitimate issues, but keep each comment concise.
|
||||||
|
|||||||
@@ -59,36 +59,31 @@ Flag if existing tests may be outdated:
|
|||||||
### Example Comment for Outdated Tests
|
### Example Comment for Outdated Tests
|
||||||
|
|
||||||
```
|
```
|
||||||
The existing tests in `src/utils/validate.test.ts` may need updates to cover the new validation rules added in this PR.
|
Tests in `src/utils/validate.test.ts` need updates for new validation rules.
|
||||||
```
|
```
|
||||||
|
|
||||||
## Comment Format
|
## Comment Format
|
||||||
|
|
||||||
### Missing Tests
|
### Missing Tests
|
||||||
|
|
||||||
Keep feedback specific and actionable:
|
Brief, actionable feedback only:
|
||||||
|
|
||||||
```
|
```
|
||||||
Consider adding tests for the changes in `src/utils/validate.ts`.
|
Missing tests for `src/utils/validate.ts`:
|
||||||
Suggested coverage:
|
- Valid input case
|
||||||
- Valid input returns expected output
|
- Invalid input error
|
||||||
- Invalid input throws appropriate error
|
- Empty string edge case
|
||||||
- Edge case: empty string handling
|
|
||||||
```
|
```
|
||||||
|
|
||||||
### Sufficient Coverage
|
### Sufficient Coverage
|
||||||
|
|
||||||
```
|
**DO NOT COMMENT.** Silence means approval.
|
||||||
LGTM
|
|
||||||
```
|
|
||||||
|
|
||||||
### Outdated Tests
|
### Outdated Tests
|
||||||
|
|
||||||
```
|
```
|
||||||
The tests in `[test file]` may need updates to reflect the changes to `[source file]`.
|
Tests in `[test file]` need updates for `[source file]` changes:
|
||||||
Specifically:
|
- [specific change]
|
||||||
- [what behavior changed]
|
|
||||||
- [what test assertions may be affected]
|
|
||||||
```
|
```
|
||||||
|
|
||||||
## Test Quality Indicators
|
## Test Quality Indicators
|
||||||
|
|||||||
Reference in New Issue
Block a user