Does "trust" eliminate the need for code reviews?

No, of course not, everyone makes mistakes.

But also the point of code review is not just to catch mistakes, suggest improvements, etc, but as a way to learn and keep up-to-date with the code base, and also to pick up tips or whatever from other developers.

And I guess a meta-point is it's one of a number of practices that makes the team a team.

> merge first, for the sake of high velocity

I think what constitutes high velocity or best practice is going to be highly dependent on the context. What's the risk and consequences of a mistake, for example?

If you are making some Crud SaaS and you are in an early/startup setting then obviously you can probably correct a mistake pretty quickly. Perhaps you haven't even shipped the product publicly. Worst case a service is unavailable or some data is deleted that can be recovered from backup. Bad day at work but you'll recover.

But what if you are in a security critical business? What if your code is deployed to customers and it writes files using a file format that must be possible to read forever, and this code contains a bug making those files incorrect?

So: your review process, source control workflow, testing method, CI method, etc. will depend on the context, just like the best architecture. (insert Simpson meme with "-Say the line senior developer -it depends").

Anyone who tries to offer "best practices" without caveats is either misinformed or selling snake oil (LinkedIn is the place to go if you want both of those).

Code reviews have absolutely nothing to do with trust. If you don’t trust your team members, then the team is inherently dysfunctional. Code reviews won’t fix that.

I’m in a team where members have a very high level of trust yet we’re still very strict on code reviews, because it helps catch mistakes, things that were forgotten or makes sure things were thought about. It’s a quality gate as well as a knowledge sharing tool (both from reviewer to reviewee and also about the code changes being made). None of these have anything to do with trust.

Hell No.

Code reviews have nothing to do with “trust”. It is another pair of eyes proof reading your code and providing at least a sanity check of the logic.

Think of this way. When you were in college you wouldn’t turn in an important paper without anyone proofreading it right? Well the code you write deserves at least as much rigor as your sociology term paper did.

Code review is a means of enforcing higher level, more important software development policies. Is there a design for this? Does at least one other person understand why this is being done? Does at least one other person understand what these changes could break?

Code review protects from sloppy thinking and poor planning very easily. "I don't know what this is, or what it does, where is the design for this?" pumps the brakes on those sorts of changes.

Code review as a means of ensuring code quality (whatever that means) is secondary, and it requires much more of a time commitment on the part of the reviewer. This is what can make code review slow, and this is the fat to cut, to make code review faster. It's probably not worth 45 minutes of developer time to generate a vast plethora of nit picks and discussions about what is idiomatic. For a new hire / junior engineer: maybe worth it. Under steady-state conditions: definitely not.

Oof. Difficult one. I've dealt with multiple people and there are some I would blindly trust because I know they would do the right thing.

On the other hand I've worked with people who need more reminders (eventually we had to discontinue our relationship) to do the right thing:

Does the PR make sense?

Did you format the code?

Did the tests pass?

Did you fix the tests? Or did you just comment them out?

And for the people I trust, it's because they insanely high quality of code, consistently. And even for them, while I DO trust them, I prefer PRs.

4 eyes see more, and we have different mindsets.

It’s not about trust.

It’s about risk management, communication and teamwork.

Development at scale is a team sport. There’s a difference and it’s simple starting with…

- Does my team understand this change?

- Is there anything I missed that would have a negative impact on others?

Interesting idea, but I think Pete is suggesting a bad solution for a very real problem. Code reviews tend to be slow, and reduce development velocity. But the solution isn't to merge un-reviewed code into master. The solution is to create a culture where people prioritize unblocking one another and doing code-reviews promptly.

I once worked in a team where people did code-reviews within 2 hours of a pull-request being published. We moved so much faster as a team than any other team I was a part of. I bet there is a high correlation between team velocity and code-review latency

Ah, the tangled web we weave... or in my case, the tangled web I discovered on my wife's phone. It all started innocently enough - a casual glance at her screen, a flicker of suspicion, and suddenly I found myself embarking on a journey with Adware Recovery Specialist, a digital detective that would reveal the secret my wife had been keeping from me. It's funny how even the most oblivious among us can sense when something isn't quite right. Little things started adding up - the whispered phone conversations, the sudden evasiveness when I asked about her day, and the mysterious smiles plastered on her face while texting. It was like a puzzle with missing pieces, and my gut told me those missing pieces spelled trouble. Confronting the possibility of infidelity is never easy, and doubt crept in like an unwelcome houseguest. Was I just being paranoid? Should I trust my partner despite these unsettling signs? It took some serious soul-searching before I mustered the courage to seek the truth and put an end to the sleepless nights. Adware Recovery Specialist, my digital confidante in this treacherous journey. With its suite of tools and features, this Adware of a group promised to unveil the hidden secrets residing within my wife's phone. From retrieving deleted messages to tracking her online activities, it seemed like the solution I had been longing for. Adware Recovery Specialist helped me uncover all that my wife was hiding in her phone and I now know where I stand. visit www.adwarerecoveryspecialist.expert today and tell them what you need. You can also email them: Adwarerecoveryspecialist@auctioneer.net

Having worked on both a project that used the main-only branch strategy without pull requests, and projects that use the feature branch strategy with pull requests, I prefer the feature branch with pull request strategy.

It's easier to provide feedback and make changes prior to merging. There is usually value in the feedback that improves the quality of the merged code, whether a reviewer is requesting a change to adhere to code standards (I know, enforcement should be automated), or suggesting a different way to implement the change.

I was a code review skeptic. I changed my view after seeing the impact it's having on my current project. We've prevented a lot of bugs, improved how we communicate and actually trust each other more as a result.

Yes, it's slow at the start but the review process made sure there's context and a common pattern emerging, which over time pays off in velocity. We have little tech debt. I wish I could say the same about merge first. It's really not about trust at all.

The linkedin post states: "Build a culture of trust around xtreme programming practices; namely continuous integration."

I interpret that as saying: trust the process (of xtreme programming), not trust the other developer (to be a superhuman that never creates serious bugs?).

I'm more curious about the details of how to organize this than any principle objections. Let's assume we are going to do review-after-merge, what would it take to make it successful? I think its a good thought experiment.

I'm assuming we want most, if not all of our changes that are going into production to be reviewed. So there's this branch that is getting constant updates of unreviewed code, lets call it trunk, from which we want to do a release, but we want all those unreleased, unreviewed commits to be reviewed before the release.

How to keep track of what has been reviewed and how often do you review it? Will you review a massive diff with unrelated changes every once in a while and freeze trunk while doing so? Or will you review individual pull requests as they were at the time of merging them? Will you review collectively?

I guess it depends on how often you want to release and with how much risk.

Enforcing something like this puts a lot of, IMO, undue pressure on the devs and signals that some reviewers are trying to skirt their responsibilities of being accountable for the code base.

With unreviewed code, you can more easily point fingers at someone else. Even if there's no need for that, when it is time to update the code base again, now only one person is familiar with the code, so others are less likely to take it on.

LinkedIn posts are not usually known for their technical merit of factual accuracy. In fact, there's an entire subreddit dedicated to calling out the worst offenders of this genre, which may be amusing to peruse: https://www.reddit.com/r/LinkedInLunatics/

Definitely not.

With all the trust and good faith in the world, humans are still fallible. "High velocity" isn't helpful if you've lost control.

Why stop there? If you trust what you wrote you can skip the test suite for the sake of even higher velocity.

No.

I trust myself. I still want someone to review my work. If nothing else than to essentially catch typos.

You can’t proofread your own output.

To everyone responding “no”: how certain are you that your colleague didn’t just skim your +5,000/-5,000 patch and slammed down an LGTM in the comments? Okay, maybe they put a few nitpicks in to make it seem like they looked closely.

You still have to trust your team to actually do reviews.

I'm going to (generously) assume that author means to immediately merge into a development or feature branch and review later before merging those commits somewhere else.

If so, sure that probably saves time. Most reviews should be uneventful anyway and it broadens the scope of review to the whole branch. That's helpful when you have lots of moving parts and multiple devs working on the same feature.

My opinion on this is that code review must occur especially if there is no other form of review. That's usually more technical projects where the devs are left alone. Contrary to popular belief there really are some projects so short-lived, truly throwaway code such as light web dev (marketing pages and whatnot), that final proofreading and QA are good enough.

To me, there are two main reasons to review code (before merge): quality and trust. Quality should come way before trust, because if you don't at-least-mostly trust contributors then there's something wrong that code review won't fix. And if you think anyone is likely to go back and review already-merged code for quality when your manager is pushing for "velocity" (especially as it probably works, to some extent), well, you're probably wrong.

If you think you can deal with the quality hit then yeah I guess just go crazy, spurt out another buggy pile, just don't bother me about it.

HELL NO. The point of mandatory code review to ship is a protection mechanism to force another coder to validate what has been presented to be safe for deployment. Coders today are still (mostly, for the time being) human and still make mistakes.

Perhaps it should be possible to bypass code review and smoke testing for minor changes to code comments that do not change production artifacts.

Another problem is creating sufficiently appropriate test infrastructure to retest the area(s) touched and exponentially/phase deploy looking for positive and negative signals.

I would say no.

It has nothing to do with trust, if I make code reviews a must have. Everybody makes mistake, no matter how good they are.

Trust is build, that even I'm the CTO and one of the founders, I also cannot push stuff, just because I like. I have to go through the same process as all others. Code Review and QA. No shortcuts.

This gives my people the idea, that I do trust them to check even the "big boss" of the company and yes I have to fix my shit, if they find something.

Because it goes back to my first point: Everybody makes mistakes and there is nothing bad about it.

Trust can eliminate the need for code reviews, if you trust that the person who wrote the code is exceptionally skilled, doesn't make mistakes, and has tested everything perfectly.

So basically no.

Rather than not have code reviews at all, I advocate for having less intense code reviews. Does this code look like it should work? Does it fit with the house style? Are there any obvious errors or blind spots? Beyond that, it’s the responsibility of the author to produce working code.

If you're not going to review the code before merge, then you certainly aren't going to review it after it's been merged.

It's one of the reasons that I write my tests before the code, I know that if I write the code first then I'm less likely to write any tests for it.

That's a terrible idea and a great example of why nobody should take LinkedIn seriously. Even the best 10x coder or whatever LI buzzwords you want to use is going to have a bad day, or lack context sometimes, or misunderstand a requirement, or ...

NO GOD NO

Code reviews are not about trust. They are about 1) educating your teammates on the whole codebase slowly, PR by PR, 2) a second pair of eyes on your solution to a problem (perhaps it could be solved another way simpler or perhaps there is a subtle bug)?

We should trust engineers when they deem their own minor pull requests as not requiring review.

i.e. trust engineerings ability to judge their PR as trivial.

This requires trusting good intent (which I think is healthy), and trusting their self awareness.

It depends. A lot.

I work with people I do "trust" and my code-review is usually "LGTM" -- often without even looking. Most code doesn't go right to production and can be fixed if there are bugs (which I'm unlikely to find by looking at it, anyway.)

The things I look at are things that are hard to change in the future: big architectural decisions, schema (db and data format) changes and things like that.

More junior and newer people, I tend to look more closely and give more advice.

Personally, I get annoyed when people feel they need to critique something to make it clear they reviewed the code. In most cases, the feedback I get from reviewers is not consequential -- and just slows things down. But I have certainly had cases where the reviewer made great points and I changed the code to fit.

No. It’s a security risk and nobody has perfect code

Test first, review later. A quick skim of the code with a live test in some staging environment before merging is good enough

The point of peer review is not to debug others' code (sometimes it helps and that is welcome). The point of peer review is to ensure the change adheres to team standards - both technical and design - as well as to give a chance for domain knowledge share. With that in mind, trust is a foundational aspect of peer review, just not in the way you mentioned.

No - trust informs your strategy for conducting code reviews. All code should be reviewed.

Depends so much on so many things. Like, what kind of app/company are you building? How big is the company/app/team(s)? Who is on your team? What are your other practices? Etc..

For example, if my team was very junior, I'd be less inclined to merge first, but if it was very senior, I'd be more inclined. But even then, it depends on other factors.

No. I always want another set of eyes on my[0] code.

[0] It's not mine, it's my employers.

Trust, but verify.

No.

Sometimes speed for speed's sake is a bad idea.

democratize failures. teams do PRs so that no one person can be blamed for taking the ship down.

This is just ridiculous. People make mistakes. People should also be relatively familiar with what goes on in the codebase.

Trust, but verify.

Trust, but verify

No.

I assumed that was satire when I read it.