chore: add review skill

[zhaohuabing]

adds review skill for coding agents #8231

[netlify]

✅ Deploy Preview for cerulean-figolla-1f9435 canceled.

Name	Link
🔨 Latest commit	7a7ebaf
🔍 Latest deploy log	https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/69dc5058991dc600087e1eba

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.35%. Comparing base (f404a9c) to head (7a7ebaf).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8237      +/-   ##
==========================================
- Coverage   74.35%   74.35%   -0.01%     
==========================================
  Files         244      244              
  Lines       38792    38872      +80     
==========================================
+ Hits        28845    28904      +59     
- Misses       7950     7965      +15     
- Partials     1997     2003       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jukie]

An AI review for the AI skill:

Suggested Additions

Consider adding these sections to make the template more comprehensive:

1. Breaking changes and versioning (.agents/skills/pr-review/SKILL.md:49)

   • Add guidance on identifying breaking changes
   • Version compatibility matrix (K8s versions, Envoy versions)
   • Migration guide requirements for breaking changes

2. Observability and metrics (.agents/skills/pr-review/SKILL.md:48)

   • New/changed metrics
   • Logging levels and structured logging
   • Tracing implications

3. Helm chart changes (.agents/skills/pr-review/SKILL.md:87)

   • Chart version bumps
   • Values.yaml backward compatibility
   • Upgrade path testing

4. Dependency updates (.agents/skills/pr-review/SKILL.md:87)

   • CVE fixes
   • Go module/Envoy proxy version updates
   • License compatibility

5. Feature flags and experimental features (.agents/skills/pr-review/SKILL.md:49)

   • Proper gating of experimental features
   • Documentation of stability level

6. Additional file-specific constraints (.agents/skills/pr-review/SKILL.md:115)

   • The "no Map in internal/ir/xds.go" constraint suggests there may be other file-specific rules worth documenting
   • Consider a centralized list or reference to architecture decision records (ADRs)

Minor Issues

1. Self-referencing example (.agents/skills/pr-review/SKILL.md:11)

   • The example references PR chore: add review skill #8237, which is this PR itself. Consider using a different PR number or a generic placeholder like #NNNN

2. Local diff workflow (.agents/skills/pr-review/SKILL.md:12)

   • The git diff example could benefit from more context about when to use local diff vs PR URL

Checklist Items

Pass:

• ✅ No debug artifacts
• ✅ Structure is clear and actionable
• ✅ Project-specific constraints are included
• ✅ References to official documentation

Recommendations:

• Consider adding examples of good vs bad review comments
• Consider adding timing guidance (when to escalate vs when to approve with comments)
• Consider adding guidance on how to handle draft PRs vs ready-for-review PRs

Overall Assessment

This is a solid foundation for a PR review skill template. The suggested additions would make it more comprehensive, but the current version is already quite usable and project-specific. The structure is logical and the emphasis on actionable, severity-ordered feedback is excellent.

[arkodg]

thanks @zhaohuabing, some more thoughts

can we add some more prescriptive suggestions like

• changes under /api

  • make sure they align with https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md
  • try to add kubebuilder and CEL validations to catch errors
  • make sure these are tested in /test/cel-validation
  • split up API and implementation in separate PRs
  • try and reuse existing types
  • keep naming consistent to this project

• changes under /internal/gatewayapi

  • errors are surfaced in status
  • add or modify a test in https://github.com/envoyproxy/gateway/tree/main/internal/gatewayapi/testdata to make sure there's coverage

• changes under /internal/xds/translator

  • add or modify a test in https://github.com/envoyproxy/gateway/tree/main/internal/xds/translator/testdata to makes sure there's coverage

• new feature

  • add an e2e https://github.com/envoyproxy/gateway/tree/main/test/e2e

[github-actions]

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, when it's ready. Thank you for your contributions!

[zhaohuabing]

@arkodg @jukie I’ve removed the pieces that I use locally such as the instruction on how to pull a PR, and only kept just the essential parts in the review skill. I think this is a good starting point, and we can improve it along the way.

[arkodg]

thanks ! this is a great start

[zhaohuabing]

/retest

[kkk777-7]

LGTM, thanks!