Add "review" claude skill and use it in "gitbhub-pr-review"#172797
Add "review" claude skill and use it in "gitbhub-pr-review"#172797abmantis wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR factors shared review guidance out of the GitHub PR reviewer skill into a new general-purpose review skill, aiming to keep PR-review behavior consistent while enabling reuse outside GitHub PRs.
Changes:
- Added a new
.claudeskill (review) that defines general code review criteria, verification steps, and an output format. - Updated
github-pr-reviewerto delegate review behavior/instructions to the newreviewskill while keeping the GitHub-specific workflow steps.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| .claude/skills/review/SKILL.md | Introduces a reusable, general-purpose review skill with guidance and an output format. |
| .claude/skills/github-pr-reviewer/SKILL.md | Simplifies PR reviewer instructions by referencing the new review skill for review behavior. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
justanotherariel
left a comment
There was a problem hiding this comment.
Adding a clause to exclude formatting/lint-only issues is probably also not a bad idea.
| - [CRITICAL] sensor.py:143 - Memory leak | ||
| - [PROBLEM] data_processing.py:87 - Inefficient algorithm | ||
| - [SUGGESTION] test_init.py:45 - Improve x variable name |
There was a problem hiding this comment.
| - [CRITICAL] sensor.py:143 - Memory leak | |
| - [PROBLEM] data_processing.py:87 - Inefficient algorithm | |
| - [SUGGESTION] test_init.py:45 - Improve x variable name | |
| - [CRITICAL] rel/path/to/sensor.py:143 - Memory leak | |
| - [PROBLEM] rel/path/to/data_processing.py:87 - Inefficient algorithm | |
| - [SUGGESTION] rel/path/to/test_init.py:45 - Improve x variable name |
Claude sometimes has the tendency to only write the filename, without the relative path to it. This is annoying because VSCode can detect filenames and can open them via CTRL+LClick, but doesn't know which e.g. __init__.py file to open.
There was a problem hiding this comment.
This was intentional eheh. It was making the summary harder to read when using the full path. Since you usually are working on a specific integration, you don't need the full path for files.
I've changed it now to be a markdown reference, so it still shows up nicely and should allow you to click in vscode (I haven't tested it since I don't use claude in vscode).
…nto review_skill_split
Proposed change
This moves some of the review instructions in the PR review skill into a general-purpose review skill.
The PR review skill should still work the same.
This is the first step in trying to create a skill that contributors can also use to review their own PRs before submitting them, aligned with our requirements.
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: