> Pull requests don't prevent bugs.

I remember once I was giving a talk on code review and I said that code review doesn't prevent bugs and people were shaking their heads. Everyone hangs on to this one thing, but if you look at the majority of pull requests that have been approved you'll see a lack of comments about pontential bugs. Also, if you do see a comment about bugs they'll often be disregarded. This drives me up the wall when I've previously pointed out bugs and they've been found to be actual bugs and had to be fixed.

I think the biggest problem with code reviews is it often becomes adversarial and people want to come out on top. I've had it more than once that someone has suggested another way of doing something, when I asked what the benefit of that way was or what was the downside of the way I was doing it no answer was forth coming however I was expected to implement their way. This becomes enraging when you point out flaws in their way but they can't find any benefit of their way.

As well as, often people just don't want to do the extra work. You point about a bunch of small improvements. It's a pain.

You end up with people asking for code style changes even tho there is a code style and the changes they're asking for aren't in those guidelines-

In my opinion, code review is a massive waste of time that is often done purely as cargo cult. People know it's a good thing to do but people have no idea how to code review and what is and what is not benefical during code review.

> I remember once I was giving a talk on code review and I said that code review doesn't prevent bugs and people were shaking their heads. Everyone hangs on to this one thing, but if you look at the majority of pull requests that have been approved you'll see a lack of comments about pontential bugs.

that's because the UX for github PRs sucks. Use Gerrit instead. It's not as pretty but it's super productive.

> I think the biggest problem with code reviews is it often becomes adversarial and people want to come out on top. I've had it more than once that someone has suggested another way of doing something, when I asked what the benefit of that way was or what was the downside of the way I was doing it no answer was forth coming however I was expected to implement their way. This becomes enraging when you point out flaws in their way but they can't find any benefit of their way.

that's not a problem caused by code review, it's caused by people who don't know how to work with others (or otherwise simply refuse to). to the degree that the code review process shines a light on this, that's good news, because that's a people problem that should be fixed.

> As well as, often people just don't want to do the extra work. You point about a bunch of small improvements. It's a pain.

when you look at a code review and you see problems in what's there, catching them in the code review is great, because that means less chance of having to fix it later.

if you see a code review and nothing's wrong with it, you just approve it. sometimes you can note things as "nits", meaning, fine for now but maybe next time do it this way.

> You end up with people asking for code style changes even tho there is a code style and the changes they're asking for aren't in those guidelines-

syntactical style and formatting should be automated, if not with tooling that actually rearranges the code in that way (e.g. [black](https://github.com/psf/black) for Python) then at least including well tuned linter rules. We use black, my own zimports tool for sorting imports, flake8 with four or five plugins as well as "black check", so we spend exactly zero time dealing with anything to do with style or code formatting, including having to even type any of it. if people are fighting over code style then there need to be tools that lock all of that down so there's no disagreements.

> People know it's a good thing to do but people have no idea how to code review and what is and what is not benefical during code review.

I work on the Openstack project and that is where I learned to do code review with Gerrit, it is completely essential for large projects and is an enormous productivity enhancement. Decades ago before CVS was even invented, the notion of using source control seemed crazy to me. That position was obviously insane, even though we were stuck using Visual Source Safe at the time. That's how it feels to me to be doing a real project today without code review and formatting tooling in place. Like even for myself, if working alone, for a large project with lots of changes I still use a code review tool, just to review myself.

That won't affect the quality of what people find and comment about.

> that's not a problem caused by code review, it's caused by people who don't know how to work with others (or otherwise simply refuse to). to the degree that the code review process shines a light on this, that's good news, because that's a people problem that should be fixed.

You say that like fixing people is an easy thing to do. It's the hardest thing to do. And the fact I've seen this at so many companies/teams leads me to believe that it's an extremely common thing.

> when you look at a code review and you see problems in what's there, catching them in the code review is great, because that means less chance of having to fix it later. > if you see a code review and nothing's wrong with it, you just approve it. sometimes you can note things as "nits", meaning, fine for now but maybe next time do it this way.

Yea... Those aren't getting done at 9/10 companies. They're getting ignored.

> syntactical style and formatting should be automated, if not with tooling that actually rearranges the code in that way (e.g. [black](https://github.com/psf/black) for Python) then at least including well tuned linter rules. We use black, my own zimports tool for sorting imports, flake8 with four or five plugins as well as "black check", so we spend exactly zero time dealing with anything to do with style or code formatting, including having to even type any of it. if people are fighting over code style then there need to be tools that lock all of that down so there's no disagreements.

Yea, we have those, I still get asked for stupid ass code style changes at every company for the last 8-years. Why? Because people can spot them and understand them. But can't code review for shit. They can't understand the idioms for the programming language, they can't understand how certain paradims work, they don't pay attention to root causes of bugs, they don't know how to design a database, they don't pay attention to performance, etc. I would say 80% of the industry can't code review for crap. And that is the biggest problem and because they can't do that all the other problems are just symptons.

> I work on the Openstack project and that is where I learned to do code review with Gerrit, it is completely essential for large projects and is an enormous productivity enhancement. Decades ago before CVS was even invented, the notion of using source control seemed crazy to me. That position was obviously insane, even though we were stuck using Visual Source Safe at the time. That's how it feels to me to be doing a real project today without code review and formatting tooling in place. Like even for myself, if working alone, for a large project with lots of changes I still use a code review tool, just to review myself.

This doesn't dispute or refute my point. It merely says in some cases it's used well. It doesn't remove the fact that majority of the time it's cargo cult and the majority of people who code review can't code review for crap or the fact that a high percentage of people just skim code review.