Live data from Hacker News

No code reviews by default

raycast.com

71–80 of 315 posts

Re: No code reviews by default

#71
Code review in real world means peer review, it's the PEER part that matters, not the CODE part.

I do think collaboration matters here, not about trust or code.

Good software comes from good collaboration.

Re: No code reviews by default

#72
post #55
post #50

Earlier quoted context omitted.

I agree that code review is not a great medium to disseminate information about the codebase, but assigning individual ownership and accountability doesn't make the need to share information go away. Ownership and accountability are workable as long as the same person sticks around long enough to fix all their mistakes. If that person leaves, the code becomes orphaned and it's up to one of their teammates to find out…

No it does not. But it also leads to one-two clear person you know to ask. It also leads to same person explaining same thing multiple times. So as long as he cares just a little, the explanation will improve. And the person doing explanation actually understand the whole part. That is big one too - you are not explained bits of it by someone who knows only small parts of it. And the system was not changed by third p…

In my mind at least, the issue with relying on on ownership, especially in a smaller team with limited resources, is that the owner isn't just the person who authoritatively understands that subsystem. The owner ends up being assigned _all_ of the work on that subsystem because it's "their responsibility".

As a result, any work that's assigned to someone else will only interact with at the interfaces / boundaries. And of course, that's sort of the intention with modularity / loose coupling - it's not necessary to learn the gory details of the XYZ subsystem's implementation, only its API. Knowing the details is certainly valuable, but there's no baseline impetus to get the owner to explain the details.

Taking over from someone who's ceding responsibility, but staying at the company, I agree that's manageable. But sometimes people get laid off suddenly (or perhaps get hit by a bus, etc) and there's no chance to get the one person who has it all in their head to explain it to you.

You mentioned high turnover, but if anything, a place with high turnover has less information locked up in a single person's head. If Alice who's worked at the company for 25 years and has always owned the code leaves, you're going to have a much harder time getting all of the tribal knowledge out of her head on two weeks notice compared to Bob who only made it 6 months.

To me, the value of code review is that even at its worst, someone other than the author is forced to look at the code. No process can force a reviewer to take an active interest in a particular subsystem, but at least we can make them look at pieces of it and see if it gets them curious enough to ask further questions of the author and better understand the system.

For myself, knowing that someone else will have to understand my code, even through the limited lens of a code review, makes me more diligent about making sure that the code is clean and the pull request has enough context that someone playing code archeologist will be able to make sense of it, even if I'm not walking them through it in real time. I will admit that this is not a universal thing - I've worked with coworkers where no matter what process is in place, they won't do the basic courtesy of reading their own changelists to see that they accidentally committed random junk files from their local machine.

I agree that good documentation around requirements is valuable. I find pull requests / code reviews are a great opportunity to highlight how the code relates to specific requirements, since the focus is narrower.

Re: No code reviews by default

#73

Earlier quoted context omitted.

> I think it is unquestionable that code-reviews catch bugs. Yes, it does catch some, but does it catch more than no code review? There is no point in catching bugs if it also creates bugs to catch.

If it catches ANY bugs it is objectively better than no PR/code-review from a bug standpoint. Unless you're arguing that pr/code-review creates more bugs than it solves? Again, I think the question is value. Is a dev's time best used in code-review vs. something else? It's almost certainly the case that time is better spent elsewhere depending on the develop and needs or the organization.

I think the question is the same one as "do bike helmets reduce bike injuries?" I.e. without code reviews, is everyone more careful? I can't think of any proper academic research that answers the question.

In my anecdotal experience, there is at least one class of bugs that code reviews are good at catching that a large expenditure of self-review effort often doesn't catch: security bugs.

Re: No code reviews by default

#74

Earlier quoted context omitted.

> I think it is unquestionable that code-reviews catch bugs. Yes, it does catch some, but does it catch more than no code review? There is no point in catching bugs if it also creates bugs to catch.

Sorry, are you saying that it's possible that code reviews cause more bugs than they solve? That's absurd...

Why would it be absurd? If there is a process to reduce risks, then people take more risks. I have been guilty of submitting code review when I'm not 100% sure it's perfect, just because I know that there is a code review process. So this definitely exists, not sure how common it is though.

Re: No code reviews by default

#75
I can see their point of view but it sounds like they focus on the downsides of code review and not the benefits.

Code reviews increases awareness of changes, helps with desiloing, gives junior devs a chance to see how others write code in their own time, reduces chance of mistakes, encourages collaboration.

To stop the endless ping ponging back and forth I advocate for Must/Should/Could in all code reviews. So long as everyone gets it, it massively speeds up code review and increases team happiness around the process.

https://careerswitchtocoding.com/blog/moscow-the-best-code-r...

Re: No code reviews by default

#76

Earlier quoted context omitted.

> I think it is unquestionable that code-reviews catch bugs. Yes, it does catch some, but does it catch more than no code review? There is no point in catching bugs if it also creates bugs to catch.

If it catches ANY bugs it is objectively better than no PR/code-review from a bug standpoint. Unless you're arguing that pr/code-review creates more bugs than it solves? Again, I think the question is value. Is a dev's time best used in code-review vs. something else? It's almost certainly the case that time is better spent elsewhere depending on the develop and needs or the organization.

> If it catches ANY bugs it is objectively better than no PR/code-review from a bug standpoint.

No, that's false. That's only true if you also assume that people write the same quality code whether there is a code review process or not. Which might or might not be true, no idea.

Re: No code reviews by default

#77
My current company (dinosaur insurance company) has a one developer per service mapping right now and it is absolute HEAVEN on Earth.

The best services rise to the top, and the other ones are refactored until they are ready to be consumed.

Re: No code reviews by default

#78
post #7

He mentions "trust" a lot. Is that why we do code reviews, because we don't trust each other? I find this attitude problematic. We do code reviews because humans make mistakes. Requirements can be misinterpreted. Different work in progress can be in conflict with each other. Reviews are a good way to learn from each other and keep abreast of what work is occurring outside your own bubble. Not doing code reviews becau…

I require PRs with code review on projects I am an admin on because I don’t trust myself. I sure as hell don’t trust other people!

Re: No code reviews by default

#79
I guess it depends a lot on the company's dev culture or team dynamic.

In my current company, our code reviews are essentially a second set of eyes by a peer. When things are "flagged" in code review it's never confrontational. Mainly they provide insight for those who haven't worked on the code base as long as to "why" things are the way they are, and it can open up discussions for improvements (or at least things to ponder for the backlog.) The odd time neat language or platform tricks are also learned.

I've worked in other places though where code reviews are done by rockstars or ninjas just looking for a reason to criticize or find fault with another's code so they can stroke their own ego. And if their code is ever questioned, expect a tantrum. Toxic environments like these say more about the company and people there than code reviews though.

Looking forward to the follow-up "no QA by default".

Re: No code reviews by default

#80

Earlier quoted context omitted.

> I think it is unquestionable that code-reviews catch bugs. Yes, it does catch some, but does it catch more than no code review? There is no point in catching bugs if it also creates bugs to catch.

Sorry, are you saying that it's possible that code reviews cause more bugs than they solve? That's absurd...

Perhaps unlikely. Or really surprising. But not absurd.

Imagine some reviewer who rejects all code reviews that don't have some recognizable pattern in them from the gang of four book. Fully complete and working code gets hacked up last minute to accommodate this person who has more seniority than sense.

Or ... maybe someone who always want there to be a single return in every function. Or heck, someone who demands that you always do early return. Either way they demand that some carefully crafted code is swapped over to the equivalent dual. And during that transformation a mistake is make that nobody notices.

I think the point is that the science way to indicate that code reviews work is to actually do the experiment. Instead of just saying, "why that's absurd that a code review could cause more defects."

I mean isn't that how hand washing got introduced into medicine? "Hey everyone, let's try washing our hands!" "Why, the hands of a gentleman are always clean. It's absurd to suggest that we should wash our hands."

Post reply on HN