From 7fd9b7314a11dcce958760d92806b53dab25fa4d Mon Sep 17 00:00:00 2001 From: openhands Date: Thu, 23 Apr 2026 17:32:34 +0000 Subject: [PATCH] =?UTF-8?q?fix(code-review):=20drop=20=F0=9F=9F=A2=20Accep?= =?UTF-8?q?table/Nit=20inline=20comments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the green Acceptable and Nit priority labels from the github-pr-review skill and add explicit guidance in both the github-pr-review and code-review skills to never post inline comments for code that is fine. These comments create review threads that must be resolved without providing any actionable value — pure noise for the PR author. Co-authored-by: openhands --- skills/code-review/SKILL.md | 1 + skills/github-pr-review/SKILL.md | 9 ++++----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/skills/code-review/SKILL.md b/skills/code-review/SKILL.md index 36d16d67..20aae13d 100644 --- a/skills/code-review/SKILL.md +++ b/skills/code-review/SKILL.md @@ -138,6 +138,7 @@ Then provide analysis (skip if 🟢): **[STYLE NOTES]** (Skip most of these - only mention if it genuinely hurts maintainability) - Generally skip style comments. Linters exist for a reason. +- Do NOT post comments for code that is acceptable or fine. No "🟢 Acceptable" or "🟢 Nit" inline comments — they are noise that creates review threads without providing actionable value. If code is good, just don't comment on it. **[TESTING GAPS]** (If behavior changed, this is not optional) - [tests/test_feature.py, Line E] **Mocks Aren't Tests**: You're only asserting mocked calls. Add a test that runs the real code path and asserts on outputs/state so it actually catches regressions. diff --git a/skills/github-pr-review/SKILL.md b/skills/github-pr-review/SKILL.md index 349665c1..7ba363cb 100644 --- a/skills/github-pr-review/SKILL.md +++ b/skills/github-pr-review/SKILL.md @@ -87,9 +87,8 @@ Start each comment with a priority label. **Minimize nits** - leave minor style | 🔴 **Critical** | Must fix: security vulnerabilities, bugs, data loss risks | | 🟠 **Important** | Should fix: logic errors, performance issues, missing error handling | | 🟡 **Suggestion** | Worth considering: significant improvements to clarity or maintainability | -| 🟢 **Nit** | Optional: minor style preferences (use sparingly) | -| 🟢 **Acceptable** | Pragmatic choice: acknowledged trade-off that is reasonable given constraints or out of scope for this PR | +**Do NOT post 🟢 Nit or 🟢 Acceptable comments.** If code is fine, simply don't comment on it. Inline comments that say "this looks good" or "acceptable trade-off" are noise — they create review threads that must be resolved without providing actionable value. **Example:** ``` @@ -142,8 +141,8 @@ curl -X POST \ 1. Analyze the code and identify important issues (minimize nits) 2. Write review data to a JSON file (e.g., `/tmp/review.json`) 3. Post **ONE** review using `gh api --input /tmp/review.json` -4. Use priority labels (🔴🟠🟡🟢) on every comment -5. Mark pragmatic trade-offs as 🟢 **Acceptable** - don't block PRs for out-of-scope improvements +4. Use priority labels (🔴🟠🟡) on every comment +5. Do NOT post comments for code that is acceptable — only comment when action is needed 6. Use suggestion syntax for concrete code changes 7. Keep the review body brief (details go in inline comments) -8. If no issues: post a short message +8. If no issues: post a short approval message with no inline comments