Yeah when we first started, "code review" was a weekly meeting of pretty much the entire dev team (maybe 10 people). Not all commits were reviewed, it was random and the developer would be notified a couple of days in advance that his code was chosen for review so that he could prepare to demo and defend it.

Am old enough that this was status quo for part of my career, and have also been in some groups that did this as a rejection of modern code review techniques.

There are pros & cons to both sides. As you point out it's quite expensive in terms of time to do the in person style. Getting several people together is a big hassle. I've found that the code reviews themselves, and what people get out of them, are wildly different though. In person code reviews have been much more holistic in my experience, sometimes bordering on bigger picture planning. And much better as a learning tool for other people involved. Whereas the diff style online code review tends to be more focused on the immediate concerns.

There's not a right or wrong answer between those tradeoffs, but people need to realize they're not the same thing.

I would guess that 3 part code review would actually be most effective. Likely even save on costs. First part is walkthrough on call, next independent review and comments. Then per need an other call over fixes or discussion.

Probably spend more time on it, but would share the understanding and alignment.

Pair programming? That is realtime code review by another human

And yet... is it? Realtime means real discussion, and opportunity to align ever so slightly on a common standard (which we should write down!), and an opportunity to share tacit knowledge.

It also increases the coverage area of code that each developer is at least somewhat familiar with.

On a side note, I would love if the default was for these code reviews to be recorded. That way 2 years later when I am asked to modify some module that no one has touched in that span, I could at least watch the code review and gleem something about how/why this was architect-ed the way it was.

Fagan inspection has entered the room

> Do junior developers not take any pride in their work?

Yes. I have lost count of the number of PRs that have come to me where the developer added random blank lines and deleted others from code that was not even in the file they were supposed to be working in.

I'm with you -- I review my own PRs just to make sure I didn't inadvertently include something that would make me look sloppy. I smoke test it, I write comments explaining the rationale, etc. But one of my core personality traits (mostly causing me pain, but useful in this instance) is how much I loathe being wrong, especially for silly reasons. Some people are very comfortable with just throwing stuff at the wall to see if it'll stick.

Many are just doing SWE for the money.

Their goal is to pass the hot potato to someone else, so they can say in the standup "oh I'm waiting on review" making it not their problem.

> I want to be sure my code works before I submit it for review.

No kidding. I mean, "it works" is table stakes, to the point I can't even imagine going to review without having tested things locally at least to be confident in my changes. The self-review for me is to force me to digest my whole patch and make sure I haven't left a bunch of TODO comments or sloppy POC code in the branch. I'd be embarrassed to get caught leaving commented code in my branch - I'd be mortified if somehow I submitted a PR that just straight up didn't work.

It’s cultural. It always seemed natural to me, until I joined a team that treated review as some compliance checkbox that had nothing to do with the real work.

Things like real review as an important part of the work requires a culture that values it.

Oh junior devs submit PRs that don't fully work all the time.