I think the other thing that often muddies the waters in discussions of code review is that open source projects and internal codebases are generally in rather different situations. An internal codebase is usually worked on by a fairly small group of experienced people, who are both creating and also reviewing PRs for it. So:
- the baseline "can I assume this person knows what they're doing?" level is higher
- making the "create PR" process take longer in order to make the review process faster is only a tradeoff of the time within the team
- if something is wrong with the committed code, the person who wrote it is going to be around to fix it
But in open source projects, there are much more often contributions by people outside the "core" long-term development team, where:
- you can't make the same assumptions that the contributor is familiar with the codebase, so you need to give things extra scrutiny
- there are often many fewer people doing the code review than there are submitting changes, so a process that requires more effort from the submitter in order to make the reviewer's job easier makes sense
- if there's a problem with the code, there's no guarantee that the submitter will be available or interested in fixing it once it's got upstream, so it's more important to catch subtle problems up front
and these tend to mean that the code-review process is tilted more towards "make it easy for reviewers, even if that requires more work from the submitter".
> - if there's a problem with the code, there's no guarantee that the submitter will be available or interested in fixing it once it's got upstream, so it's more important to catch subtle problems up front
It's also more important to have good tools to analyze subtle problems down the line, thus increasing the importance of bisection and good commit messages.
An underrated benefit of "make it easy for reviewers" is that when a bug is found, everybody becomes a potential reviewer. Thus the benefit does not finish when the PR is merged.