[hnthrow0287345]

>It's even worse than that: non-junior devs are doing it as well.

This might be unpopular, but that is seeming more like an opportunity if we want to continue allowing AI to generate code.

One of the annoying things engineers have to deal with is stopping whatever they're doing and doing a review. Obviously this gets worse if more total code is being produced.

We could eliminate that interruption by having someone doing more thorough code reviews, full-time. Someone who is not being bound by sprint deadlines and tempted to gloss over reviews to get back to their own work. Someone who has time to pull down the branch and actually run the code and lightly test things from an engineer's perspective so QA doesn't hit super obvious issues. They can also be the gatekeeper for code quality and PR quality.

[marcosdumay]

A full-time code reviewer will quickly lose touch with all practical matters and steer the codebase into some unmaintainable mess.

This is not the first time somebody had that idea.

[JohnBooty]

I've often thought this could work if the code reviewer was full-time, but rotated regularly. Just like a lot of jobs do with on-call weeks, or weeks spent as release manager - like if you have 10 engineers, and once every ten weeks it's your turn to be on call.

That would definitely solve the "code reviewer loses touch with reality" issue.

Whether it would be a net reduction in disruption, I don't know.

[necovek]

Doing code review as described (actually diving deep, testing etc) for 10 engineers producing code is likely not going to be feasible unless they are really slow.

In general, back in 2000s, a team I was on employed a simple rule to ensure reviews happen in a timely manner: once you ask for a review, you have an obligation to do 2 reviews (as we required 2 approvals on every change).

The biggest problem was when there wasn't stuff to review, so you carried "debt" over, and some never repaid it. But with a team of 15-30 people, it worked surprisingly well: no interrupts, quick response times.

It did require writing good change descriptions along with testing instructions. We also introduced diff size limits to encourage iterative development and small context when reviewing (as obviously not all 15-30 people had same deep knowledge of all the areas).

[bee_rider]

You could do some interesting layering strategies if you made it half time, for two people. Or maybe some staggered approach: each person does half time, full time, then half time again, with there people going through the sequence at a time. Make each commit require two sign-offs, and you could get a lot of review and maybe even induce some cooperation…

[kaffekaka]

"Interesting" is the word I would use as well, but also cumbersome and complicated.

[jaggederest]

I think it's amenable if you make code review a primary responsibility, but not the only responsibility. I think this is a big thing at staff+ levels, doing more than your share of code review (and other high level concerns, of course).

[kragen]

Linus Torvalds is effectively a full-time code reviewer, and so are most of his "lieutenants". It's not a new idea, as you say, but it works very well.

[sorokod]

> One of the annoying things engineers have to deal with is stopping whatever they're doing and doing a review.

I would have thought that reviewing PRs and doing it well is in the job description. You latter mention "someone" a few times - who that someone might be?

[bee_rider]

Can we make an LLM do it?

“You are a cranky senior software engineer who loves to nitpick change requests. Here are your coding standards. You only sign off of a change after you are sure it works; if you run out of compute credits before you can prove it to yourself, reject the change as too complex.”

Balance things, pit the LLMs against each other.

[postflopclarity]

I do this all the time. I pass my code into "you are a skeptic and hate all the code my student produces: here is their latest PR etc.. etc.."

[osn9363739]

I have devs that do this and we have CI AI code review. Problem is, it always finds something. So the devs that have been in the code base for a while know what to ignore, the new devs get bogged down by research. It's a net benefit as it forces them to learn, which they should be doing. It def slows them down though which goes against some of what I see about the productivity boost claims. A human reviewer with the codebase experience is still needed.

[mywittyname]

Slowing down new developers by forcing them to understand the product and context better is a good thing.

I do agree that the tool we use (code rabbit) is a little too nitpicky, but it's right way more than it's wrong.

[bee_rider]

I don’t use any of these sorts of tools, so sorry for the naive questions…

What sort of thing does it find? Bad smells (possibly known imperfections but least-bad-picks), bugs (maybe triaged), or violations of the coding guides (maybe known and waivered)?

I wonder if there’s a need for something like a RAG of known issues…

[baq]

GPT 5+ high+ review bots find consistently good issues on average for me, sometimes they’re bogus, but sometimes they’re really, really good finds. I was impressed more than once.

[jjmarr]

We do this at work and it's amazing.

[cm2012]

This would probably catch a lot of errors

[immibis]

As I read once: all that little stuff that feels like it stops you from doing your job is your job.

[gottagocode]

> We could eliminate that interruption by having someone doing more thorough code reviews, full-time. Someone who is not being bound by sprint deadlines and tempted to gloss over reviews to get back to their own work.

This is effectively my role (outside of mentoring) as a lead developer over a team of juniors we train in house. I'm not sure many engineers would enjoy a day of only reviewing, me included.

[jms703]

>One of the annoying things engineers have to deal with is stopping whatever they're doing and doing a review

Code reviews are a part of the job. Even at the junior level, an engineer should be able to figure out a reasonable time to take a break and shift efforts for a bit to handle things like code reviews.

[mywittyname]

This sounds good in theory, but in practice, a person capable of doing a good job at this role would also be a good developer whose impact would be greater if they were churning out code. This is a combination of a lead engineer and SDET.

In reality, this ends up being the job given to the weakest person on the team to keep them occupied. And it gives the rest of the team a mechanism to get away with shoddy work and not face repercussions.

Maybe I'm just jaded, but I think this approach would have horrible results.

AI code review tools are already good. That makes for a good first pass. On my team, fixing Code Rabbit's issues, or having a good reason not to is always step 1 to a PR. It catches a lot of really subtle bugs.

[shiandow]

The best way I have of knowing code is correct on all levels is convincing myself I would write it the same way.

Thr only way to be 100% sure is writing it myself. If I know some one reasonable managed to write the code I can usually take some shortcuts and only look at the code style, common gotchas etc.

Of course it wouldn't be the first time I made some erroneous assumptions about how well considered the code was. But if none of the code is the product of any intelligent thought well, I might as well stop reading and start writing. Reading code is 10x harder than writing it after all.

[necovek]

With time and experience, reading code becomes much easier.

And well-written code is usually easy to read and understand too!

The purpose of a code review is, apart from ensuring correctness, to ensure that the code that gets merged is easy to understand! And to be honest, if it's easy to understand, it's easy to ensure correctness too!

The biggest challenge I had was to distinguish between explanations needed to understand the change, and explanations needed to understand the code after it was merged in. And making it clear in my code review questions that whatever question I have, I need code and comments in the code to answer them, not the author to explain it to me (I frequently have already figured out the why, but took me longer than needed): it's not because I did not get it, it's because it should be clearer (finding the right balance between asking explicitly, offering a suggestion, or pitting it as a question to prompt some thinking is non-trivial too).

[Spoom]

Big companies would outsource this position within a year, I guarantee it. It's highly measurable which means it can be "optimized".

[nunez]

This sounds like what unit tests after every commit and e2e tests before every PR are supposed to solve.

[ohwaitnvm]

So pair programming?

[sodapopcan]

Yep, eliminates code reviews altogether. Unfortunately it remains wisely unpopular with perle even saying “AI” can be the pair.

[gaigalas]

It does not eliminate code reviews.

In practice, you should have at least one independent reviewer who did not actively worked on the PR.

That reviewer should also download the entire code, run it, make tests fail and so on.

In my experience, it's also good that this is not a fixed role "the reviewer", and a responsability everyone in the team shares (your next task should always be: review someone else's work, only pick a new thing to do if there is nothing to review).

This practice increases quality dramatically.

[sodapopcan]

> It does not eliminate code reviews.

Yes it does. There are many ways to do things, of course, and you can institute that there must be an independent reviewer, but I see this is a colossal waste of time and takes away one of the many benefits of pairing. Switch pairs frequently, and by frequently I really mean "daily," and there is no need for review. This also covers "no fixed responsibilities" you mentioned (which I absolutely agree with).

Again, there are no rules for how things must be done, but this is my experience of three straight years working this way and it was highly effective.

[gaigalas]

Mixed-level pairs (senior/junior), for example, are more about mentoring than reviewing. Those sessions do not qualify for "more than one pair of eyes".

Excited (or maybe even stubborn) developers can often win their pairs by exhaustion, leading to "whatever you want" low effort contributions.

Pairs tend to under-document. They share an understanding they developed during the pairing session and forget to add important information or details to the PR or documentation channels.

I'm glad it has been working for you. Maybe you work in a stellar team that doesn't have those issues. However, there are many scenarios that benefit a lot from an independent reviewer.

[sodapopcan]

The problem with most people's experience with pairing is that they are given no guidance on how it should be done outside of "just work with someone."

In terms of under-documentation, I didn't really find that. Most of my jobs have either been way under-documented, way over-documented (no one reads it and it gets outdated). Again I'll say that I find switching pairs daily is key with no one person stay on for more than 2 days in a row (so if a story takes three days to complete, the two people who finished it are not the same two who started it). This keeps that internal knowledge well-spread.

But you're right, if you're dealing with overly excited/stubborn folk who refuse to play ball, that's obviously not going to work. Conversely, if you have a trusting team and someone is having one of those days where they feel useless and unmotivated, pairing can turn it into some kind of useful day. This could be because your colleague gets you excited or, if you have high trust, I've literally said and had people say, "I'm not feeling it today... can you drive all day?" and you're still able to offer good support as a navigator and not have a total write-off of a day.

To the point of improving an otherwise bad day, one of the more interesting arguments against pairing I've heard is that companies use it to make sure everyone is always working. That would indeed be sad if that was the motivation.

[necovek]

Pairing people together on a single task makes that task get done faster, with higher quality. However, when paired together, the people still pick up some of the same biases and hold the same assumptions and context, so it is really worse than having a single author + independent reviewer.

So:

  single author, no review < pair programming, no review < single author + review < pair programming + review

[sodapopcan]

Again, if you rotate pairs daily this doesn't happen. You could argue that creates a "team bias" but I call that cohesion.

[aslakhellesoy]

Shhh don’t let them know!