4 ms·

What to Put in a Claude Code Skill for Reviewing Your Team's Code

9 points by parad0x0n 7mo ago

rgambee 7mo ago
I definitely see a lot of these anti-patterns in the code that CC writes. Many of these can be caught at the time the code is written without needing to wait for a PR review. To me, it seems like most of these instructions belong in CLAUDE.md instead of or in addition to a specialized review skill. Are there things in the review skill that don't belong in CLAUDE.md?
- parad0x0n 7mo ago
  fair point. I think they should ALSO be in your CLAUDE.md. Doing Claude reviews with the exact same instructions still makes sense because the PR itself is often a co-production between AI and the human who gives the instructions (and in the coding process the agent might be more sycophantic toward the user's ideas). Additionally, when writing code, Claude is more focused on writing code that works and puts a lot of your repo into context, while during reviewing it pays more attention to these guidelines
- Bullhorn9268 7mo ago
  Weirdly, CC still adds stupid comments to guide its next steps despite having that instruction in CLAUDE.md AFAIK.
mckennameyer 7mo ago
Aren't vibe PRs way more likely to get abandoned? Sure they reduce reviewer load, but then everyone feels less urgency to do a human review after. Do you think the skill is making that better or worse?
- parad0x0n 7mo ago
  yeah I guess figuring out how AI and your team can optimally work together is not that straightforward.. probably every engineering team is trying to figure that out atm :D but if we already let AI write reviews, they should at least be as good as they can
nbosse 7mo ago
> Auto-review on routine PRs produced too much noise. A one-line config change doesn't need a 12-point review Shouldn't you prompt the review to just be a one-line review then? I see real danger in having humans review "the easy fixes" manually, but then rely on Claude for the complicated stuff. I'd be curious to learn whether you have similar prompts and skills for producing code, as opposed to just code review.
- parad0x0n 7mo ago
  what danger?
  - Bullhorn9268 7mo ago
    Maybe like that the humans would not properly review the more complicated ones too? I don't think it's a big danger to be clear.
peter94 7mo ago
What's the point of having the testing be done by Claude Code via a skill, rather than just hard-coding the set of tests to be run?
- parad0x0n 7mo ago
  that's another option of course. But it's definitely easier to setup all these checks and linter tests with a skill file vs git hooks and actions
- Bullhorn9268 7mo ago
  Some of the rules are impossible to code generally enough, right? Like, "no defensive coding" - I can't imagine what kind of AST shenaningas you would have to do without basically banning `if`.
tliptay 7mo ago
Is there any lesson here for non-devs (like me) who are using CC to write some pretty long, gnarly notebooks. The problem I have recently run into is that CC vibe codes really long code with lots of checks. But, then when I want to modify it, CC has more code to read and it gets overwhelmed. If it didn't include as many checks to begin with, it would be easier to understand what is going on. Any advice? Have you seen this with code in production?
- Bullhorn9268 7mo ago
  I mean, have you tried to ask it not to do those checks? It's similar to the OP about no defensive coding. In general, CC works better when you can tell it WHAT TO DO as opposed what NOT to do, but sometimes there is no way around it. What kind of checks it's adding? Like asserting that columns exist and stuff?

9 points by parad0x0n 7mo ago

I definitely see a lot of these anti-patterns in the code that CC writes. Many of these can be caught at the time the code is written without needing to wait for a PR review. To me, it seems like most of these instructions belong in CLAUDE.md instead of or in addition to a specialized review skill. Are there things in the review skill that don't belong in CLAUDE.md?

parad0x0n 7mo ago
fair point. I think they should ALSO be in your CLAUDE.md. Doing Claude reviews with the exact same instructions still makes sense because the PR itself is often a co-production between AI and the human who gives the instructions (and in the coding process the agent might be more sycophantic toward the user's ideas). Additionally, when writing code, Claude is more focused on writing code that works and puts a lot of your repo into context, while during reviewing it pays more attention to these guidelines
Bullhorn9268 7mo ago
Weirdly, CC still adds stupid comments to guide its next steps despite having that instruction in CLAUDE.md AFAIK.

mckennameyer 7mo ago

Aren't vibe PRs way more likely to get abandoned? Sure they reduce reviewer load, but then everyone feels less urgency to do a human review after. Do you think the skill is making that better or worse?

parad0x0n 7mo ago
yeah I guess figuring out how AI and your team can optimally work together is not that straightforward.. probably every engineering team is trying to figure that out atm :D but if we already let AI write reviews, they should at least be as good as they can

nbosse 7mo ago

> Auto-review on routine PRs produced too much noise. A one-line config change doesn't need a 12-point review Shouldn't you prompt the review to just be a one-line review then? I see real danger in having humans review "the easy fixes" manually, but then rely on Claude for the complicated stuff. I'd be curious to learn whether you have similar prompts and skills for producing code, as opposed to just code review.

parad0x0n 7mo ago
what danger?
- Bullhorn9268 7mo ago
  Maybe like that the humans would not properly review the more complicated ones too? I don't think it's a big danger to be clear.

peter94 7mo ago

What's the point of having the testing be done by Claude Code via a skill, rather than just hard-coding the set of tests to be run?

parad0x0n 7mo ago
that's another option of course. But it's definitely easier to setup all these checks and linter tests with a skill file vs git hooks and actions
Bullhorn9268 7mo ago
Some of the rules are impossible to code generally enough, right? Like, "no defensive coding" - I can't imagine what kind of AST shenaningas you would have to do without basically banning `if`.

tliptay 7mo ago

Is there any lesson here for non-devs (like me) who are using CC to write some pretty long, gnarly notebooks. The problem I have recently run into is that CC vibe codes really long code with lots of checks. But, then when I want to modify it, CC has more code to read and it gets overwhelmed. If it didn't include as many checks to begin with, it would be easier to understand what is going on. Any advice? Have you seen this with code in production?

Bullhorn9268 7mo ago
I mean, have you tried to ask it not to do those checks? It's similar to the OP about no defensive coding. In general, CC works better when you can tell it WHAT TO DO as opposed what NOT to do, but sometimes there is no way around it. What kind of checks it's adding? Like asserting that columns exist and stuff?