With regards to the given list, I think #2 would be the most forgiving, in the sense that #1 suggests that code reviews are viewed solely negatively and are punishable if undertaken. But that minor quibble aside, I have some questions about what each of these would even look like.

For example, #3 seems to be that code can be committed and pushed, and then review sought after-the-fact, but any results of the code review would not be binding for the original commit author to fix, nor apparently tracked for being fixed later. If that’s a correct description, I would describe that as the procedurally worst of the bunch, since it expends the effort to do reviews but then has such an open-loop process that the results of the review can be swept under the rug.

On the note of procedure, it is always preferable to have closed loops, where defects are: a) found, b) described, c) triaged, d) assigned or deferred, e) eventually fixed, and f) verified and closed out. At least with your examples #1 and #2, they don’t even bother to undertake any of the steps for a closed loop. But #3 is the worst because it stops right after the first step.

Your example #4 is a bit better, since in order to keep to a specified timeframe, the issues found during review have to at least be recorded in some fashion, which can satisfy the closed-loop steps, however abbreviated. If the project timeline doesn’t allow for getting all the code review issues fixed, then that’s a Won’t Fix and can be perfectly reasonable. The issue has been described and a risk decision (hopefully) was made to ship with the issue anyway. All things in life are a balance of expected risk to expected benefit. Ideally, only the trivial issues would be marked as Won’t Fix.

But still, this means that #4 will eventually accumulate coding debt. And as with all debt, interest will accrue until it is either paid down or the organization succumbs to code insolvency, paralyzed because the dust under the rug is so large that it jams the door shut. I hope you’ll allow me to use two analogies in tandem.

Finally, there is #5 which is the only one that prevents merging code that still has known issues. No doubt, code that is merged can still have further bugs that weren’t immediately obvious during code review. But the benefit is that #5 maintains a standard of functionality on the main branch. Whereas #4 would wilfully allow the main branch to deteriorate, in the name of expediency.

No large organization can permit any single commit to halt all forward progress on the project, so it becomes imperative to keep the main branch healthy. At a minimum, that means the branch can be built. A bit higher would be to check for specific functionality as part of automated checks that run alongside the code review. Again, #4 would allow breaking changes onto the branch due to expediency, whereas #5 will block breaking changes until either addressed, abandoned, or a risk decision is made and communicated to everyone working on the project to merge the code anyway.

TL;DR: software engineering processes seek to keep as many people working and out of each other’s way as possible, but it necessarily requires following steps that might seem like red-tape and TPS reports

Ah, I see that OP added more details while I was still writing mine. Specifically, the detail about having only a group of 5 fairly-experienced engineers.

In that case, the question still has to focus on what is an acceptable risk and how risk decisions are made. After all, that’s the other half of code reviews: first is to identify something that doesn’t work, and second is to assess if it’s impactful or worth fixing.

As I said before, different projects have different definitions of acceptability. A startup is more amenable to shipping some rather ugly code, if their success criteria is simply to have a working proof of concept for VCs to gawk at. But a military contractor that is financially on the hook for broken code would need to be risk-adverse. Such a contractor might impose a two-person rule (ie all code must have been looked at by at least two pairs of eyeballs, the first being the author and the second being someone competent to review it).

In your scenario, you need to identify: 1) what your success criteria is, 2) what sort of bugs could threaten your success criteria, 3) which person or persons can make the determination that a bug falls into that must-fix category.

On that note, I’ve worked in organizations that extended the two-person rule to also be a two-person sign-offs: if during review, both persons find a bug but also agree that the bug won’t impact the success criteria, they can sign off on it and it’ll go in.

Separately, I’ve been in an organization that allows anyone to voice a negative opinion during a code review, and that will block the code from merging until either that person is suitably convinced that their objections are ameliorated, or until a manager’s manager steps in and makes the risk decision themselves.

And there’s probably all levels in between those two. Maybe somewhere has a 3-person sign-off rule. Or there’s a place that only allows people with 2+ years of experience to block code from merging. But that’s the rub: the process should match how much risk is acceptable for the project.

Boeing, the maker of the 737 MAX jetliner that had a falty MCAS behavior, probably should use a more conservative process than, say, a tech startup that makes IoT devices. But even a tech startup could be on the hook for millions if their devices mishandle data in contravention to data protection laws like the EU’s GDPR or California’s CCPA. So sometimes certain parts of a codebase will be comparmentalized and be subject to higher scrutiny, because of bugs that are big enough to end the organization.