Skip to content

Commit 0b4cc5f

Browse files
Bordapre-commit-ci[bot]Copilot
authored
Refine Copilot and CONTRIBUTING guidelines for clarity and conciseness (#2150)
* Refactor Copilot and CONTRIBUTING guidelines for clarity and conciseness * Streamline and condense PR review guidelines in CONTRIBUTING.md for improved clarity and usability. * Apply suggestions from code review * fix(pre_commit): 🎨 auto format pre-commit hooks --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 0ed48f0 commit 0b4cc5f

File tree

2 files changed

+179
-288
lines changed

2 files changed

+179
-288
lines changed

.github/CONTRIBUTING.md

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,100 @@ To run tests with coverage:
286286
uv run pytest --cov=supervision
287287
```
288288

289+
## 🔍 PR Review Guidelines
290+
291+
These guidelines help reviewers provide consistent, actionable feedback efficiently. Your goals: validate completeness, identify risks, provide actionable feedback, and highlight quality gaps.
292+
293+
### Overall Recommendation
294+
295+
Start with a clear recommendation using these levels:
296+
297+
- 🟢 **Approve** — Ready to merge
298+
- 🟡 **Minor Suggestions** — Improvements recommended but not blocking
299+
- 🟠 **Request Changes** — Must address issues before merge
300+
- 🔴 **Block** — Critical issues require major rework
301+
302+
Example: `🟠 Request Changes — Missing unit tests for PolygonMerger and no mkdocs entry.`
303+
304+
### PR Completeness
305+
306+
Verify requirements are met (✅ Complete / ⚠️ Incomplete / ❌ Missing / 🔵 N/A):
307+
308+
- [ ] Clear description of what changed and why
309+
- [ ] Tests added/updated for new functionality or bug fixes
310+
- [ ] Docstrings follow [Google-style](https://google.github.io/styleguide/pyguide.html#383-functions-and-methods)
311+
- [ ] Docs entry added to mkdocs (new functions/classes only)
312+
- [ ] Google Colab provided (if demonstrating feature/fix)
313+
- [ ] Screenshots/videos included (visual changes only)
314+
315+
Call out missing items explicitly in your review.
316+
317+
### Quality Scores
318+
319+
Use **n/5 scoring** with inline code comments for specifics:
320+
321+
**Code Quality (n/5):**
322+
323+
- 5/5 🟢 Excellent — 4/5 🟢 Good — 3/5 🟡 Acceptable — 2/5 🟠 Needs Work — 1/5 🔴 Poor
324+
- Check: correctness (edge cases, None checks, bounds), Python best practices (idiomatic patterns, error handling, type hints), project conventions (docstrings, linting, import order, PEP 8 naming)
325+
326+
**Testing (n/5):**
327+
328+
- 5/5 🟢 Comprehensive — 4/5 🟢 Good — 3/5 🟡 Adequate — 2/5 🟠 Insufficient — 1/5 🔴 Missing
329+
- Verify: unit tests for new code, edge cases covered, specific assertions, realistic scenarios, clear test names
330+
331+
**Documentation (n/5):**
332+
333+
- 5/5 🟢 Excellent — 4/5 🟢 Good — 3/5 🟡 Adequate — 2/5 🟠 Insufficient — 1/5 🔴 Missing
334+
- Confirm: docstrings for public functions/classes, parameters/returns/exceptions documented, usage examples, mkdocs integration, changelog entry for user-facing changes
335+
336+
### Risk Assessment
337+
338+
Flag risks with severity (5/5 🔴 Critical — 4/5 🟠 High — 3/5 🟡 Medium — 2/5 🟢 Low — 1/5 🟢 Negligible):
339+
340+
**Common risk categories:**
341+
342+
1. **Breaking changes** — API changes, removed features, behavior modifications (must include migration guide)
343+
2. **Performance** — Inefficient algorithms, memory-intensive operations, bottlenecks
344+
3. **Compatibility** — New Python/dependency requirements, platform-specific code
345+
4. **Security** — Unvalidated input, code execution risks, data exposure
346+
347+
### Review Summary Template
348+
349+
```markdown
350+
## Review Summary
351+
352+
**Recommendation:** [emoji] [Status] — [justification]
353+
354+
**PR Completeness:**
355+
- ✅ Complete: [items]
356+
- ❌ Missing: [gaps]
357+
358+
**Quality Scores:**
359+
- Code: n/5 [emoji] — [reason]
360+
- Testing: n/5 [emoji] — [reason]
361+
- Documentation: n/5 [emoji] — [reason]
362+
363+
**Risk Level:** n/5 [emoji] — [description]
364+
365+
**Critical Issues (Must Fix):**
366+
1. [Issue] — See comment on `file.py`
367+
368+
**Suggestions (Optional):**
369+
1. [Improvement] — See suggestion on `file.py`
370+
371+
**Next Steps:**
372+
1. [Action item]
373+
```
374+
375+
### Review Best Practices
376+
377+
**DO:** Use inline GitHub comments with suggestions, explain *why* (not just *what*), distinguish blocking vs. nice-to-have, acknowledge good work, run linter if needed (`uv run pre-commit run --all-files`)
378+
379+
**DON'T:** Mention line numbers in summary (use inline comments), give vague feedback, nitpick style (defer to tools), assume knowledge of conventions, block on minor issues
380+
381+
**Tone:** Be respectful, specific, pragmatic, and consistent. Focus on actionable feedback that moves PRs toward merge.
382+
289383
## 📄 License
290384
291385
By contributing, you agree that your contributions will be licensed under an [MIT license](../LICENSE.md).

0 commit comments

Comments
 (0)