Code reviews aren’t just for catching bugs

> I don't see how "promiscuous pairing" helps with most of them.

Maybe you should try it! The "promiscuous" part transfers context and knowledge around more quickly, and gives more opportunities, earlier in the process, to respond to feedback from a wider variety of perspectives. Two people might come to the same conclusion that another, joining a couple of hours later and without following the same mental path, would find crazy. It's generally pretty easy to change direction after a couple of hours.

> Pairing by definition slows down all work on code(you have 1 person working instead of two).

"Work on code" is not all that's involved in shipping product. In my experience, pairing produces less/simpler/better code that more directly addresses intended business value and cuts off unproductive paths more quickly than other methods. I much much much prefer cranking out less of the right code than producing tons of "perfect" code that solves the wrong problem (slightly hyperbolic, but you get the point, and it totally happens all the time). It's worth mentioning that what I'm talking about is pairing done well, by people who have learned how to do it well. It's absolutely possible to do a shit job pairing, as with anything else.

>* They tend to encourage focus on abstract code polishing without proportionality or relation to the value of the code. >This is an issue with the priorities of the people doing the code review, and thus applies just as much to pairing.

This is true to a certain degree, but good pairing typically involves thousands of little tradeoff decisions and discussions about how to solve problems. A code review can't reproduce the richness of all of that communication, so it necessarily emphasizes code in the abstract over deep consideration of the tradeoffs involved in the solution to a problem.

> I have no idea how pairing is possibly different from code review in [the case of superficial improvements.]

When I'm sitting down with someone to solve a problem and we explore a path for a few minutes, then I can step back and say, "wait, based on what we've just done, I think we're thinking about this whole thing in the wrong way, what about this other thing," it's easy to turn around and explore a totally different approach. When somebody has sunk an entire day into solving a problem, polishing the code to impress their coworkers, and responding to code review comments, it's extremely difficult to say, "hey, I think we're thinking about this the wrong way, what about this other thing?" I've experienced this problem nearly every time I've participated in code reviews and rarely during pairing. (To that point, pairing is such a better dynamic for this type of exchange. A code review has an oppositional nature, while with pairing, you're literally on the same side of the problem, sitting together to find a solution. The dynamic of questioning the path/approach starts off in a much more natural/friendly/collaborative place.)

> so the most egregious problem with code reviews is the people that do them might have wrong priorities

I think it goes beyond that. As mentioned above, the structure of code reviews (vs. pairing) favors certain types of feedback over others, and makes the feedback that I believe to be most valuable very difficult to give, socially, and too late to be really valuable.

>A benefit of pairing that I can see over code reviews is when you get feedback - instant, real-time in pairing vs late in code reviews.

Yes! Exactly. And from what I've seen, this makes an enormous amount of difference.

I don't agree with you about the time differences, especially when considering the full cycle of delivery. It's a notoriously difficult thing to measure effectively and/or prove, so...it's one of those things you have to experience first hand (in an environment where it's done well...there are plenty where it's not) to really buy into perhaps.

It's also true that some people just want to work solo. I'm motivated by producing the best possible product and generating the best possible outcomes, and pairing is the best way I've encountered to do that, so...in general, I'd prefer not to work with those people when they're steadfastly opposed to trying collaborative work. At the same time, I've found that most people who give it a shot with a good pair who has some patience, empathy, and skill, end up loving it. Even folks who generally work alone.

I view bugs as a potential issue in an individual's development process. It sounds like the biggest reason you already hit on -- gigantic pull requests. Gigantic requests usually mean PRs that aren't focused to one discrete work-piece, but represent work that kinda meanders to completion. Make sure they're focused, and ask for nonessential pieces to be delegated to another PR (style-only PRs being a good example).

Ultimately the only way to suppress large pull requests (or problematic practices in general) is to evangelize simple development workflows that others can help you with. Keep in mind that change for anyone, no matter intelligence or stubbornness, takes time - culture especially. You need to make it easier for them to adopt your methods instead of theirs when the time comes and they get fed up and are ready to change. Make sure you don't have an antagonistic relationship, either, or they'll do anything but what you want just to spite you :(

I've broken up some stuff to look for in code reviews, in code bases, and in developers. You probably only want to look at developers.

For the code itself:

- Do we have overdeveloped patterns that get in the way of expressive code? - Do these bugs fall into a general classification that can pinpoint a source (such as timing issues, authentication, database calls, or info validation?) - Is your code-base DRY? Repetitious functionality sprinkled around will mean bugs that never get fixed. Eliminate repetition. - Age of code-base? Newer code-bases should be more accepting of large PR's/more bugs, but should compensate with refactoring-only PRs, test blitzes, and accessible code coverage. Old code may have traditions that have outlived their usefulness, and owners that have grown too accustomed to the state of the code to understand the need for improvement. - No experts. Is this an inherited code-base with poorly documented patterns whose original authors have moved on?

For code reviews (speeding up review):

- First read-through of the code is only to familiarize myself with the code added. - Does this PR solve one problem or a bunch of problems? One feature/bug/style-change == one PR (per repository). Ask to break it up if it's big and doing more than one thing (unless those things are highly coupled). - Is there core functionality? Start with the most reused piece, and critique that first. - Is this code shared? Shared code should be held to a higher standard, especially if other people depend on it. Get their eyes on it too, it takes some work off you, builds ownership, and if this person is a problem, you'll want allies to back you up. - Are methods long? Longer methods tend to do more than one thing, and invites glomming onto existing methods instead of creating your own. Higher standards for shared code modification tends to lead to shorter methods, because reusing code needs to be a deliberate decision. Plus long methods can hide code reuse. - Is manual testing difficult? Long or difficult verification loops tend to allow for more bugs.

For the developer:

- What are their tools? What tools are you using that makes you more effective? Learning new tools is hard, but most bugs come from ineffective development practices - Are they senior in knowledge or senior in age/time? Not all senior devs deserve their title. - Do they stay on-task? Do they have to thrash between tasks? Unfocused development is a big source of bugs. - How familiar are they with the code base? Senior or not, code from new devs should require more scrutiny (but not to the point that it discourages collaboration).