Add Wasp code review skill - #4556
Conversation
GitHub Actions
Inactive
@wasp.sh/spec
@wasp.sh/wasp-cli
@wasp.sh/wasp-cli-darwin-arm64-unknown
@wasp.sh/wasp-cli-darwin-x64-unknown
@wasp.sh/wasp-cli-linux-arm64-glibc
@wasp.sh/wasp-cli-linux-x64-glibc
@wasp.sh/wasp-cli-linux-x64-musl
commit: |
|
|
||
| - Demand precise, informative names. Names must not misdirect, omit relevant behavior, or assume unavailable context. | ||
| - Make names accurately reflect what their declarations represent. | ||
| - Context is kind: evaluate names within their scope and surrounding vocabulary. |
There was a problem hiding this comment.
| - Context is kind: evaluate names within their scope and surrounding vocabulary. | |
| - Context is king: evaluate names within their scope and surrounding vocabulary. |
| ## Contracts And Interfaces | ||
|
|
||
| - Review names before implementation details. Poor names often expose deeper problems in decomposition and architecture. | ||
| - Declaration should be enough to understand, without the implementation. A function’s name, arguments, types, and contract should explain its behavior. |
There was a problem hiding this comment.
This one feels like it could be too much. But we can tone it down in the future if needed.
| - Demand code to be easy to read. Understanding one section should not require reading the entire file. | ||
| - Review from the perspective of a developer with minimal reasonable context. | ||
|
|
||
| ## Design And Architecture |
There was a problem hiding this comment.
I would also add something like
"Avoid NIH": before creating a new abstraction or creating a helper, look in the codebase or existing dependencies if one already exists. You can also bring in established or industry-standard libraries where it makes sense. If you end up writing new code and it might be reusable in different places in code, put it in a common location."
| - Use comments only for information that names, arguments, and types cannot express. | ||
| - Remove comments that merely narrate obvious code or exhibit generated filler. | ||
| - Suggest pruning fluff and no-op sentences from prose. | ||
|
|
There was a problem hiding this comment.
"Remove comments that only clarify questions or edge cases from a specific session and that are obvious or not needed for a fresh reviewer."
|
|
||
| ## Code Smells | ||
|
|
||
| <!-- Based on Martin Fowler's Refactoring --> |
There was a problem hiding this comment.
| <!-- Based on Martin Fowler's Refactoring --> |
We don't need the agent to know that
| ## Other | ||
|
|
||
| - Check whether requested improvements justify their implementation cost. | ||
| - Run `shellcheck` when shell scripts change. |
There was a problem hiding this comment.
Remove. Reminding it to use linters should be a write-time thing, not review-time. Instead we might need to add shellcheck to our linters.
FranjoMindek
left a comment
FranjoMindek
left a comment
There was a problem hiding this comment.
Didn't read the sources
| description: Review code changes for correctness, clarity, and potential issues. Use when the user asks for a review, code review, PR review, or wants feedback on a diff or file. | ||
| --- | ||
|
|
||
| Review the specified PR, diff, or files. If none are specified, review the staged and unstaged changes. |
There was a problem hiding this comment.
If none are specified, review the staged and unstaged changes.
Definitely needs to be part of the description.
This comes from your action workflow.
We don't have such assumptions.
|
|
||
| # Findings And Reporting | ||
|
|
||
| - Make every finding actionable. Include the file and line, explain the problem, and suggest a concrete fix. |
There was a problem hiding this comment.
Maybe instruct it to add examples e.g. code snippets?
Can be valid for both "explain the problem" and "suggest a concrete fix".
3 participants
Description
Adds
/wasp-review, our team code review skill. It's based on some of our existing writing on reviewing and some other existing skills.The skill lives in
.agents/skillsand is linked from.claude/skillsso Claude can find it too.Sources
Type of change
Checklist
I tested my change in a Wasp app to verify that it works as intended.
🧪 Tests and apps:
examples/kitchen-sink/e2e-tests.waspc/data/Cli/templates, as needed.examples/, as needed.examples/tutorials) I updated the tutorial in the docs (and vice versa).📜 Documentation:
web/docs/.🆕 Changelog: (if change is more than just code/docs improvement)
waspc/ChangeLog.mdwith a user-friendly description of the change.web/docs/migration-guides/.versioninwaspc/waspc.cabalto reflect the changes I introduced.No automated tests were added because this is only an instruction file.