Man, 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??
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.
At another level, code reviews also give you the time to really think about what is being built and why. I've often come across looming architectural or strategic failures simply by having to stare at a bunch of code and wonder why it works a certain way. Same goes for writing the code by hand too, though it's harder to see past your own biases and ideas sometimes. I find code reviews deeply valuable for myself and my team.
> Essentially, the kinds of places that bugs like to hide and grow in.
I like an ecological-framing, especially as systems get more complex.
Yes, I could hunt cockroaches all day, but it would be even better to have caught how the new wall was missing a metal screen over a low ventilation opening.
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]