Thread

There is more to code review than (automatable) detection

165 points · 117 comments · utiiiD

n4r9 · 1790545492 · focus · HN ↗
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: <a href="https://mathstodon.xyz/@mjd/115096720350507897" rel="nofollow">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.
1. aeonik · 1790547600 · focus · HN ↗
  Missing my biggest issues as you ask the agents to do larger tasks with less up front planning.
  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.
  1. bunderbunder · 1790553582 · focus · HN ↗
    “Net new” is one it seems to be particularly bad at.
    I don’t think I have ever even once seen an LLM solve a problem related to overengineering by simply removing the overengineering. They always choose to add more epicycles and further compound the complexity.
    sfn42 · 1790578594 · focus · HN ↗
    You can tell it to do that. I don't mean abstractly, I mean concretely, tell it how to solve it and it will do what you tell it.
    classified · 1790581030 · focus · HN ↗
    When you get to that level of micromanagement, won't it be simpler to just do it yourself?
    sfn42 · 1790581722 · focus · HN ↗
    Some times I do, but no not really. If it's just a simple change of one or two lines I'll do it manually, but larger changes are much faster with AI.
    I'll lay out a vague plan and let the AI fill in the blanks, mostly it'll get them right and where it doesn't I'll just tell it to do it differently and how. Works great for me.

Open on Hacker News to reply ↗

Unofficial Hacker News client; not affiliated with Y Combinator.