There's been a lot of talk about the purpose of code review recently. It makes sense in the face of AI. Heres a link that was submitted a little while ago: https://mathstodon.xyz/@mjd/115096720350507897
And in response I wrote a non-exhaustive checklist of things that a code review can look for:
- Does it functionally achieve what it sets out to (as per tacker issue or PR description)?
- Does it have extraneous code? Leftover debug prints, private API keys etc...
- Does it have any obvious defects? Memory leaks, un-handled edge cases, security flaws, obsolete API calls, etc...
- Could it be more understandable? Add/remove abstractions, better variable/method names, more/less functional etc...
- Is the style consistent with the codebase and/or style guidelines?
- Are there obvious performance improvements? Hashset instead of list, lazy evaluations, etc...
- Is it sufficiently well tested?
I think LLMs are okay at most of these, and worst at the first.
code reviews are just a gateway that can be whatever you want it to be, and is kind of legacy human coder thing now. At its basics it was a point to catch problems that humans were likely to make / would more likely make if they knew there wasn't a review. Now you can target it for AI mistakes. You can build your code review skills (AI skill) to be incredibly thorough. The points made in the article don't really seem like things you need to do at the "legacy" gateway of code review. Things are different now. Code is cheap. Validation, Product Coherence, Governance need to be done early and throughout.
I actually think this might be a false positive. I think it got tripped up by the higher than average use of jargon (which I don't mind here because the article itself flowed well and raised good points).
Gpt-zero scores "human", and I've always found it to be a better judge
And in response I wrote a non-exhaustive checklist of things that a code review can look for:
- Does it functionally achieve what it sets out to (as per tacker issue or PR description)?
- Does it have extraneous code? Leftover debug prints, private API keys etc...
- Does it have any obvious defects? Memory leaks, un-handled edge cases, security flaws, obsolete API calls, etc...
- Could it be more understandable? Add/remove abstractions, better variable/method names, more/less functional etc...
- Is the style consistent with the codebase and/or style guidelines?
- Are there obvious performance improvements? Hashset instead of list, lazy evaluations, etc...
- Is it sufficiently well tested?
I think LLMs are okay at most of these, and worst at the first.
Is there already a pattern or code on in in the existing codebase that handles this functionality,
Do we really need net new code to achieve this functionality?
Can existing code be extended or abstracted to more cleanly implement this feature or functionality.
- Is the change architecturally right?
Particularly the latter LLMs seem still pretty useless at.
Although I do think that LLMs have made it much easier to justify writing low-value code which can make this more common now.
„The indent is wrong here“
„Comments should end with a period“
Because this kind of feedback is and was always easy.
Gpt-zero scores "human", and I've always found it to be a better judge
Something is missing in the new ai bot review paradigm we’ve all sleepwalked into.
I’ve been building Archme.io for this reason. PR reviews for the age of AI