Live data from Hacker News

Code review can be better

tigerbeetle.com

31–40 of 253 posts

Re: Code review can be better

#31
While I like the post and agree with everything the author talked about I find that this is not my problem. Despite having a similar workflow (classic vim user). The problem I have and I think a lot of others have too is that review just doesn't actually exist. LGTMs are not reviews, yet so common.

I'm not sure there's even a tech solution to this class of problems and it is down to culture. LGTMs exist because it satisfies the "letter of the law" but not the spirit. Classic bureaucracy problem combined with classic engineer problems. It feels like there are simple solutions but LGTMs are a hack. You try to solve this by requiring reviews but LGTMs are just a hack to that. Fundamentally you just can't measure the quality of a review[0]. Us techie types and bureaucrats have a similar failure mode: we like measurements. But a measurement of any kind is meaningless without context. Part of the problem is that businesses treat reviewing as a second class citizen. It's not "actual work" so shouldn't be given preference, which excuses the LGTM style reviews. Us engineers are used to looking at metrics without context and get lulled into a false sense of security, or convince ourselves that we can find a tech solution to this stuff. I'm sure someone's going to propose a LLM reviewer and hey, it might help, but it won't address the root problems. The only way to get good code reviews is for them to be done by someone capable of writing the code in the first place. Until the LLMs can do all the coding they won't make this problem go away, even if they can improve upon the LGTM bar. But that's barely a bar, it's sitting on the floor.

The problem is cultural. The problem is that code reviews are just as essential to the process as writing the code itself. You'll notice that companies that do good code review already do this. Then it is about making this easier to do! Reducing friction is something that should happen and we should work on, but you could make it all trivial and it wouldn't make code reviews better if they aren't treated as first class citizens.

So while I like the post and think the tech here is cool, you can't engineer your way out of a social problem. I'm not saying "don't solve engineering problems that exist in the same space" but I'm making the comment because I think it is easy to ignore the social problem by focusing on the engineering problem(s). I mean the engineering problems are magnitudes easier lol. But let's be real, avoiding addressing this, and similar, problems only adds debt. I don't know what the solution is[1], but I think we need to talk about it.

[0] Then there's the dual to LGTM! Code reviews exist and are detailed but petty and overly nitpicky. This is also hacky, but in a very different way. It is a misunderstanding of what review (or quality control) is. There's always room for criticism as nothing you do, ever, will be perfect. But finding problems is the easy part. The hard part is figuring out what problems are important and how to properly triage them. It doesn't take a genius to complain, but it does take an expert to critique. That's why the dual can even be more harmful as it slows progress needlessly and encourages the classic nerdy petty bickering over inconsequential nuances or over unknowns (as opposed to important nuances and known unknowns). If QC sees their jobs as finding problems and/or their bosses measure their performance based on how many problems they find then there's a steady state solution as the devs write code with the intentional errors that QC can pick up on, so they fulfill their metric of finding issues, and can also easily be fixed. This also matches the letter but not the spirit. This is why AI won't be able to step in without having the capacity of writing the code in the first place, which solves the entire problem by making it go away (even if agents are doing this process).

[1] Nothing said here actually presents a solution. Yes, I say "treat them as first class citizens" but that's not a solution. Anyone trying to say this, or similar things, is a solution is refusing to look at all the complexities that exist. It's as obtuse as saying "creating a search engine is easy. All you need to do is index all (or most) of the sites across the web." There's so much more to the problem. It's easy to over simplify these types of issues, which is a big part of why they still exist.

Re: Code review can be better

#32
post #26

Earlier quoted context omitted.

It's better, but still quite deep vendor lock-in (in both GitHub and VSCode).

JetBrains IDEs can do the same.

Unfortunately it’s not feature complete - you can’t paste images in review comments, for example. Still very useful for large PRs though.

Re: Code review can be better

#34

Just taking a step back, it is SO COOL to me to be reading about stacked pull requests on HN. When we started graphite.dev years ago that was a workflow most developers had never heard of unless they had previously been at FB / Google. Fun to see how fast code review can change over 3-4yrs :)

I'm a pre-mercurial arcanist refugee who tends to promote Graphite in teams that are still struggling with mega-PRs and merge commits and other own goal GitHub-isms. Big fan in general even with the somewhat rocky scaling road we've been on :)

And I very much appreciate both the ambition and results that come from making it interop with PRs, its a nightmare problem and its pretty damned amazing it works at all, let alone most of the time.

I would strongly lobby for a prescriptive mode where Graphite initializes a repository with hardcore settings that would allow it to make more assumptions about the underlying repo (merge commits, you know the list better than I do).

I think that's what could let it be bulletproof.

Re: Code review can be better

#35
If you want to remain relevant in the AI-enabled software engineering future, you MUST get very good at reviewing code that you did not write.

AI can already write very good code. I have led teams of senior+ software engineers for many years. AI can write better code than most of them can at this point.

Educational establishments MUST prioritize teaching code review skills, and other high-level leadership skills.

Re: Code review can be better

#36

If you want to remain relevant in the AI-enabled software engineering future, you MUST get very good at reviewing code that you did not write. AI can already write very good code. I have led teams of senior+ software engineers for many years. AI can write better code than most of them can at this point. Educational establishments MUST prioritize teaching code review skills, and other high-level leadership skills.

> AI can already write very good code

Debatable, with same experience, depends on the language, existing patterns, code base, base prompts, and complexity of a task

Re: Code review can be better

#37

Just taking a step back, it is SO COOL to me to be reading about stacked pull requests on HN. When we started graphite.dev years ago that was a workflow most developers had never heard of unless they had previously been at FB / Google. Fun to see how fast code review can change over 3-4yrs :)

I'm a pre-mercurial arcanist refugee who tends to promote Graphite in teams that are still struggling with mega-PRs and merge commits and other own goal GitHub-isms. Big fan in general even with the somewhat rocky scaling road we've been on :) And I very much appreciate both the ambition and results that come from making it interop with PRs, its a nightmare problem and its pretty damned amazing it works at all, let a…

We've talked about a "safe mode" where we initialize it similar to JJ - such that you can no longer directly run git commands without funneling them thru graphite, but which would make it bulletproof. Would that be interesting?

Re: Code review can be better

#38

If you want to remain relevant in the AI-enabled software engineering future, you MUST get very good at reviewing code that you did not write. AI can already write very good code. I have led teams of senior+ software engineers for many years. AI can write better code than most of them can at this point. Educational establishments MUST prioritize teaching code review skills, and other high-level leadership skills.

> AI can already write very good code Debatable, with same experience, depends on the language, existing patterns, code base, base prompts, and complexity of a task

How about AI can write large amounts of code that might look good out of context.

Re: Code review can be better

#39

Earlier quoted context omitted.

> AI can already write very good code Debatable, with same experience, depends on the language, existing patterns, code base, base prompts, and complexity of a task

How about AI can write large amounts of code that might look good out of context.

Yeah, LLMs can do that very well, IMO. As an experienced reviewer, the "shape" of the code shouldn't inform correctness, but it can be easy to fall into this pattern when you review code. In my experience, LLMs tend to conflate shape and correctness.

Re: Code review can be better

#40

If you want to remain relevant in the AI-enabled software engineering future, you MUST get very good at reviewing code that you did not write. AI can already write very good code. I have led teams of senior+ software engineers for many years. AI can write better code than most of them can at this point. Educational establishments MUST prioritize teaching code review skills, and other high-level leadership skills.

I’m considered one of the stronger code reviewers on the team, what grinds my gears is seeing large, obviously AI heavy PRs and finding a ton of dumb things wrong with them. Things like totally different patterns and even bugs. I’ve lost trust that the person putting up the PR has even self reviewed their own code and has verified it does what they intend.

If you’re going to use AI you have to be even more diligent and self reviewed your code, otherwise you’re being a shitty team mate.

Post reply on HN