[epolanski]

Jm2c, but by far the best place I've ever worked (my current client) we don't do code reviews at all unless the author wants feedback.

PRs are good ways to defend your code base from bad code, and they were born in open source where you literally have no clue who the contributor is, but years of experience left me convinced that I don't want such a system where there's constant need to overview each other's work.

I want to work with a team where there is high respect and trust. A team where I know I won't like or love all the decisions others make, but I trust their judgement. Maybe they did indeed hack an ugly solution cheating the type system and automated controls. So what? What matters is if they have done so for good reasons (stuff was super urgent, a proper solution was just not worth the effort as the feature/fix was really not important for the business).

This made development speed skyrocket and I'm no longer bound to infinite code reviews as if we were sending rockets on Mars.

I also want to say, code quality is high, but this stems both from working with great individuals that can be trusted and from much higher interaction speed.

[neo2006]

I don't see code review as an overwatch but a good communication tool and a way to think about problems collectively. Code review reduce bugs because it permit to have people think about the problem from multiple angles. About decisions, I think important design decisions need to be taken prior to the code review steps ,and also reviewed. I'm not sure what context you worked in that gave you that opinion about code review being a sign of distrust and I think the problem lay within the culture in those places not the code review practice it self.

[duncan-donuts]

I don’t mind code reviews so I’m not really defending who you’re replying to. There is another way to think about this tho and I actually have found this practice to be light years more effective than code reviews. Where I work we have a practice where before you start doing a ton of work on your card/ticket you should find someone on your team, explain the problem to them, and show them how you’re going to do the work. This gives your teammates an opportunity to give you feedback before anyone has done anything, and you can both poke holes in design decisions before anyone has done anything they feel strongly about. If you do this process 95% of code reviews are pretty much worthless.

[neo2006]

I fully agree, we also do this and this is the design review I alluded to. That said, I think both have different purpose. The design review is more about the approach and the high-level implementation. The code review is about the implementation details. Also, a great aspect often overlooked around code review is that it's a great teaching tool if used right. Asking question go a long way and make both the reviewer and the reviewee learn. Code review is often looked to as a senior watching over what juniors are doing. In my experience it should be done for all levels and even juniors should review seniors PR as they can bring novel perspective and at least learn from the code by reviewing it.

[epolanski]

I'm gonna rephrase my previous point. What you say makes sense, but only if you understand that it takes away time, money and energies and after knowing it, and the impact of all this time drag you determine it's worth it.

Might apply to huge products making millions $ every day. Sure, delivering a bug will be expensive.

Might apply when you can't trust your colleagues (not skilled, reasonable or experienced enough).

Might apply if your code is niche but mission critical (maybe some safety system on a car or a dangerous tool, or surgery equipment).

Of course I agree with all you said.

But you're writing some Java/TS web app which isn't raking much money, or has still to be launched, or your biggest focus is time to market to beat competitors and you're wasting time on code reviews the author hasn't requested?

In this scenario (my current client) I want to have a team I can trust and gets stuff done. This does not imply that CRs don't happen or design isn't discussed, but it happens when it brings value or it is needed. And I like it that way, where we can build stuff, rather discussing how to build it.

[neo2006]

If we want to include the cost of a code review into the equation we should also include the cost of fixing a big that made it to production which is in most of the cases higher then code review. Skill is not a factor here, I work and worked with some of the best engineers in the world and everyone write buggy code sometimes. Software engineering is a complex practice. If you are writing some prototyping code or some code that will never make it to production or in a very early stage of a startup, sure I can understand your point.

[scubbo]

I respect the thoughtfulness with which you made your point, but completely disagree. As another commenter stated - Code Reviews are, for me, primarily a method for communication and knowledge dissemination, secondarily a means to get a second pair of eyes on my work to ensure it's legible and that I didn't miss anything, and only as a distant third a means to "protect" the codebase.

[jacobegold]

Not to mention the value of a diff/pull request/changelist/patch/merge request (so many names...) and its corresponding discussion as a historical artifact for your team to reference later on!

[RHSeeger]

I trust and respect the people I work with. I also prefer to have code reviews, even (especially) of my own code. Code reviews can point out code that

- Doesn't consider an edge case (NPEs being the most common)

- Doesn't match up well with the "standard" way we do things (and, all things being equal, using the same approach in every place improves maintainability)

- Has a simpler approach

- Doesn't have a comment where it's not clear _why_ the code is doing something (that the author of the code sees as obvious, because they wrote it)

- Has misleading method/class/field names

These are all things that I've seen from both myself and others on my team. Having one other team member take a look at my code before it goes to QA can save a lot of time later.

[epolanski]

None of the things you said matters if your issue is time to market or you have little business.

Who cares about an edge case no user's gonna see?

Who cares about an edge case for a non business critical feature?

Who cares if there was a better way to implement something if it's not a priority?

Our work it's not meant to give us devs intellectual challenges but solve people's problems.

All the things you list are reasonable if you're making money or they are core to the product or they impact user's safety in any way.

Otherwise they don't matter at all.

And half the things you list are solved by having high hiring standards and paying people well.

Stuff may not be perfect, maybe there will be better ways, maybe it will be inconsistent at times but it will be more than good enough and move the business forward at much higher speed. Also, just to reiterate I've never stated that we don't have standards or make architectural decisions or don't give feedback. I merely stated that reviewing PRs for us is an exception, not a rule and that I prefer a system of trust.

[RHSeeger]

My goal is to build things that work, and can continue to be worked on and improved over time; where development doesn't grind to a halt in 6 months because nobody cares about quality.

Edge cases matter sometimes matter and sometimes do not. They particularly matter in cases where, if the happen, the leave the system in a bad state (corrupt data, etc). Having a second pair of eyes take a look at the code and think "where can this go wrong" doesn't cost much, but can save a lot of pain later.

Doing things in a simpler way and/or a way that matches the way we generally do things makes it easier to maintain. And it's fairly common for code maintenance and/or further development on the same code, to be more common than writing brand new functionality (code that doesn't touch existing code). This is directly related to the "grind to a halt" I mentioned earlier.

Literally nothing I mentioned can be solved by hiring standards or pay. Once, because hiring people new to software development is a thing. Two, because everyone... EVERYONE makes mistakes. And EVERYONE can do better with the help of their peers.

I prefer that every PR get reviewed by someone, with the (uncommon) exception of those where the developer says it isn't worth a review. It generally takes very little time, and it adds to delivered code quality.

[halostatue]

I have been in the industry for a fairly long time and I have seen code review tools pop up, and GitHub came fairly late to the table (IIRC, I was using a code review tool in 2007 against Perforce; GitHub wasn’t generally available until 2008 and I think I joined in late 2008 or early 2009).

I have been on teams where we simply merged things when complete and where we required code review before merge. In every case, the latter had better overall speed because we weren’t (usually) introducing regressions because no one had seen the code. (Pairing is a form of real-time code review, but the team I worked on that used pairing heavily still needed code review; I remember a regression was introduced even though the code had been pair-developed.)

There is, however, a case where if one works in an industry where ISO270001 or SOC2 or PCI compliance is required, recommended, whatever — you will have to have a policy on code review in order to pass.

[devin-sills]

Same experience here. Productivity unlocks like crazy, and amazingly the sky doesn't fall. It does require stringent hiring practices though and a flat, open culture. Oftentimes I don't need a formal syntactical process like a review but rather just need to bounce ideas off folks to get a good plan for a larger feature.

[quacker]

Agree. The better your team members, the less need there is for code review.

However, the approach of few/no code reviews doesn’t work for large open source projects that accept external contributions (the author of the post is mitchellh who is a founder of HashiCorp, which has/had several large open source projects).