alt Hacker NewsMan, I'm probably going to expose my own inexperience here, but like, did anybody actually expect code reviews to catch and prevent bugs on a line by line basis? Like ever?
I never have operated this way. Code reviews were a basic sanity check, a chance to implement best practices, and a chance to teach these things to juniors.
If I was concerned about something breaking, I'd duscuss test coverage.
Was I doing it wrong this whole time??
Replies
I review code line by line and build a mental model of the software as I review it. I find plenty of important bugs that way. PRs are an opportunity to learn (both committer and reviewer), communicate, and improve the quality of code a team ships. Sadly, many people decline that opportunity for the sake of short-term velocity.
No. You are on to something here. The utopia I see programmers lamenting the loss of, the one where human code review was perfect and never let any bugs slip through, never existed. I see people on this board and others complaining all the time about LLM generated spaghetti code and bug-ridden codebases. Well, I've been working those code bases for 20 years. We didn't agents to make them. LLMs are a new tool, but the problems that come with them are nothing new.
Yes I have often caught stuff like "not using DB transactions in the right way" or "this is not going to be race safe" for instance from juniors just by glancing on a PR.
I find recent LLMs don't do such mistakes though, in my experience. And also LLMs will find such mistakes if asked to review, if not perfect then more predictable than humans.
It was more important for giant C or fortran code bases. You will legit catch pointer errors or memory mgmt stuff, going line by line. Once Sun busted in with Java is when I really saw the shift to skim reviews. So much stuff is caught by compilers now, or SAST, that using dev time to find code smells is mostly a waste of their time. And it's not like the old way caught everything, it's just that the stuff we take for granted being caught by compilers or Sonarqube now, just used to get shipped to prod and cause abends and production support calls at 3am.
The best code review process I've done are more like oral exams ie: the person walks through the changes and we listen as they describe it.
It's a good way to catch thinking errors, which are the insidious errors.
Code review is like a dam holding back a huge body of water. Nobody does it perfectly and almost all software turns to shit given enough time.
I think a code review should be just that. A review of the optics of code itself not the solution or anything like that. If that is important it should be done earlier in the process before coding. Even more important when coding with AI. The reviewer is not a tester. I've heard horrible stories from friends who mainly works with fixing AI generated code that junior devs pushes at a rapid pace.
[dead]
I’ve caught bugs in code reviews before, but I would agree that it’s probably not the most crucial thing you should strive to do with code reviews.
That said, I think I’ve done a tremendous amount of PREVENTING bugs from being introduced via code review. Caught all kinds of weird code patterns, duplication, wrong levels of abstraction, etc. Essentially, the kinds of places that bugs like to hide and grow in.
I still think that’s valuable, and it’s the kind of thing that models generally aren’t great at, to this day. It generally relies on having a large amount of context about the project and about what’s being solved, so if you wanted to do this via automated review, you maybe COULD, but it would probably get pretty expensive, especially on larger projects.