Like everything in software development, it comes with "it depends".

- getting peer review on iffy code = 100%

- waiting for a PR to be approved for 6 hours, by 2 parties when it's typo fixes and adding some comments = ...

As a senior dev with over 20 years, I value code reviews from even junior devs. Heck it might spark a conversation that might mean everyone learn something, or get a refresher. It's also simply put: 2 brains are better than 1, particularly with codebase fatigue.

And by the way, in dynamic languages: even more so. Dynamic languages save you time in the writing side of things, which you pay back in the testing. Make no mistake, you are the compiler.

So my philosophy is if you feel this might benefit from another set of eyes, seek reviews. If you feel you're going to waste someone's time, don't.

But mission critical code, e.g. handling payment information, user privacy impact of any kind, business logic, should go through code reviews, on top of QA and UAT.

Trust people, don't trust code. And if you can't trust your people, you need new people(!)

My observation is that code reviews are one of the worse solutions to the problem. They are almost never done right. People tend to treat them as nuisance and not focus or focus on the wrong things. Or people try to be nice to other people and "not make problems". They are constant source of problems because they tend to crash into deadlines and usually lose. It is also frequently the first time somebody finds out somebody else spend a week writing a feature producing completely unusable mess which could have been avoided if there was human contact during development. Also reviewing large body of work after it has been "completed" is simply the worst time to have open and critical discussion about quality.

My tested and recommended process is working in pairs, rotating pairs regularly and sharing/rotating all tasks within pairs, equally. You have two people actively working together on each piece of code. They are supposed to both understand what is happening, care for quality and be vigilant to catch defects. This is also the right time to have actual input regarding quality or design improvements. And the code once it is done can immediately be committed and shipped so there is no awkward delay between code finished and deployment.

I also learned over the years pair programming is also a fantastic way to achieve some other goals like people socialising, getting to know each other professionally, learning from each other, newcomers getting bootstrapped, etc.

The author keeps equating code review with pull requests and gated check-ins. This is a fundamental mistake.

Code review is fine and necessary. The problem is when a coworker can veto your own work, rather than just review it and offer helpful suggestsions. Pull requests are simply an artifact of certain version control systems that have somehow become pervasive to our entire software methodology. It doesn't have to be that way.

It's not a question of trusting in perfection. Of course everyone makes mistakes. But do you trust your coworker to not be overly pedantic and opinionated, to not get in your way and block your progress? You shouldn't necessarily trust that either.

I strongly dislike checking in code that hasn't gone through code review.

I trust that I'm not doing anything malicious, but I am also aware that I'm a human being and make mistakes. Sometimes really dumb ones. I like running it past another pair of eyes that aren't intimately familiar with my code to minimize the amount of dumb that makes it into the final product.

There are small teams that are highly effective without PRs, given how little their owned areas intersected. Forcing the PR reviews there would only harm - any knowledge sharing is more effective at the whiteboard, anyway.

It is funny because everyone seems to have forgotten that source control versioning purpose used to be that it was cheap to revert things if needed.

We have PR only, squash merge and “must have a linked work item” set up on our default branches.

It’s not to protect our default branches from our co-workers. It’s to protect them from ourselves. I work in a small team in a non-tech enterprise organisation, and each of our smaller code-bases tend to be mainly worked on by one person. Since the things are interconnected as stuff is today, I’ll sometimes add some react bits to our major front-end app that one of my coworkers have written 90% off, and sometimes he will add new controllers or features to one of the back-end services that I have build. In those cases, the strict default branch policies have never been an issue. Most of the time I’ll approve a Pull Request from my co-workers without reviewing anything, exactly because I trust them.

What I don’t trust is for myself to not cut-corners and not alter the default branch, or especially, to create a work-item on the DevOps board for what I think is a minor fix. I don’t trust myself to do that, because sometimes, I still turn the default branch rules off when I need to tinker because the red tape is annoying. Which is fine, until it isn’t, and the more times it isn’t the more I come to respect the red tape we’ve installed to protect us from ourselves.

There's another kind of trust: I trust the organization to prioritize core competency among staff and not use me as a crutch to compensate for that lack of effort by burning all my time reviewing crap programming work.

I think the discussion around reviews becomes more informed once you're past that hurdle.

Also, as I've said before, architecture reviews yield 10x the value of reviewing programming logic.

Trust erodes, if we ongoingly have to step in & someone did it esnt seem able to eventually grok all the hurdles. So having systems that help automatically tell us of issues, that can be do the job, are really important.

It was more an organizational rather than peer level concern, but I've heard some good discussion of trust being the currency of the profession. Feeling like you are adequately regarded, even though it's likely few others really see into your part of the faxtory floor. Being able to trust the org to make good choices. The article focuses on peer relationships & risk & getting good output, but I think the social factors around trust more broadly are at least as important.

There's a lot of software being written by one or two people where there's not review. I work on a few of these style systems. It's a judgement game.

Pull requests are there for documentation, for automated quality checks and builds and deploys, for learning opportunities, and for getting other people to review it in case you missed something. This last piece may not always be the most effective. It's hard to incentivize sometimes when everyone is busy. But there are still valuable reasons to have a PR.

You can speed up the code review process by trusting people more on the basis of a longer tenure of high quality code and few bugs making it to production. Especially if they add screen shots or video snippets to the pull request to demonstrate the feature and to demonstrate edge cases.

Make it as asynchronous as possible and it scales to hardcore software engineering speed. And with time reduce the number of gatekeepers by assigning more authoritative titles with lower required numbers of approvals for things. That way we begin to promote the people who are demonstrating the most efficacy in shipping working code that gets the job done, rather than those people who are maxed out on talking a lot but don't ship any code at all. Or worse, the silent coasters who do nothing and say nothing lol. We see you now.

We view an approval on merge request as an affirmation that the approver can be called about this code in the middle of the night. In a sense, it puts other developers with skin in the game. Lack of trust is not an issue, and if it was, it would undermine far more than MRs.

Whatever you do, hire a tester. A good tester trusts no one and nothing.

If you are writing a whole article on not trusting yourself, and never even mention the idea of engaging with someone dedicated to finding problems, then I can’t take you seriously.

Good testing and code review procedures protect yourself and your colleagues from mistakes. It's not an issue of trust.

Trust, but verify. I believe in code review, nobody's perfect, and delivering broken code bc you missed something, simply sucks...

[dead]