Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Code reviews do find bugs

two-wrongs.com

Post reply on HN
Print view View original

21–30 of 169 posts

Re: Code reviews do find bugs

#21

Post by elcomet »

saulpw wrote: post #9
willio58 wrote: post #3

Agreed. I mainly manage and review code at this point in my career. I find many bugs, every once in a while finding something that would have caused an outage or notable problem for users. What I find more though is code that isn't thought through. Tech debt and code smell are real, and they affect the performance of a team. Nipping that in the bud takes quality PR reviews and time to meet with submitters around issu…

How do you phrase these warnings? "Next time.."? I have a hard time being serious with my own warnings if it's fine enough for now.

We prefix our comments with "minor:", and all employees know that this means it's something that would be nice but not necessary to merge

Re: Code reviews do find bugs

#22

Post by Mathnerd314 »

My question is, do human reviewers find more bugs than ChatGPT? Because finding a cofounder costs a lot but asking ChatGPT is free. https://www.thatsoftwaredude.com/content/12848/chatgpt-can-d... says it is mediocre, but that was a year ago and honestly mediocre code reviews seem sufficient.

Re: Code reviews do find bugs

#23

Post by spankalee »

alex_lav wrote: post #20
spankalee wrote: post #14

Earlier quoted context omitted.

Bikeshedding in a team can be good. If you're all painting the shed, it helps to agree on the color. More generally, code review is a great opportunity for incrementally gaining or encouraging alignment across the team. What the team chooses to align on and how strongly are left up to it, so hopefully they choose to not get bogged down in inconsequential details, but completely skipping the pretty cheap chance for re…

You're making a lot of positive-upsided assertions about code review. My point is there is too much opportunity for negative behavior. It's the same as everything in tech, in life, "It can be good if everyone does their part to keep it good". And yet, most don't.

"most don't" is a strong claim. In my experience, core review has been undoubtedly good. I would never run or join a company without it.

I'm writing code solo for the moment, and code review is maybe the thing I miss the most.

Re: Code reviews do find bugs

#24

Post by closeparen »

epolanski wrote: post #2

My beef with code reviews is that often they lead to tremendous amounts of wasted time, that's many thousands spent in a single week sometimes for simple pull requests. Working from 6 years, not much, and not in as many places like others, I have built the opinion that code reviews are like tests, they should be used as a tool when they are necessary, they shouldn't be the default for every change. In best case scena…

The discipline of putting up small, coherent, explained, tested, and reviewable units of change, that you have looked over and feel comfortable showing off to other people as your work product, is 80% of the value for me. Whether anyone else actually thinks about it deeply or has something useful to say about it is secondary.

Re: Code reviews do find bugs

#25

Post by lhamil64 »

jiggawatts wrote: post #6
godelski wrote: post #4

I think this is really important in that it is bigger than "code reviews." It does show how people greatly misunderstand statistics[0]. And what's even funny is at surface level the claim that code review "does nothing" __sounds__ ludicrous. But people "believe" because they are annoyed with code review, not because they "actually" believe the results. But statistics are tricky. With the example given in the article…

> support sends you to their page that requests you to "upvote" a "feature" or bug issue. Microsoft does this for enterprise products where customers might be paying $100K/mo or even millions. “We hear you, but your complaint is just not popular enough so go away.” “Sure it’s a catastrophic data loss bug that ate your finance transactions, but if other people can’t identify that their seemingly unrelated crash is the…

My company/team has very different processes for bugs vs feature requests. If a customer opens a ticket and we determine it's a bug, we will generally fix it in the reported release and later (unless it's a security vulnerability or other major problem). But for feature requests we just tell them to submit it to a community and we evaluate it to see if it's valid and something we'd likely implement given the other work we have on our plate, but not necessarily do it any time soon.

Re: Code reviews do find bugs

#26

Post by sys_64738 »

There are various levels to code reviews. Code review tools that are web based are pretty poor in my experience. Anything more than a few lines across multiple files needs a cscope type tool.

Also what type of review? Is this a prototype needing a high level design review so that the actual review doesn’t turn into a design review? How often does that occur?

Who are the reviewers and what’s the process? Key stakeholders have more influence and you need to consider the reviewer’s experience, knowledge and credibility.

Finally how important is the code? Is it kernel code, or high execution daemon code needing race condition and memory leak checking? Are you using static analysis for the code? Does the code even compile and do what it is designed to do? Where are the unit test logs?

Lots to consider.

Re: Code reviews do find bugs

#27

Post by ecshafer »

jellyfishbeaver wrote: post #11
willio58 wrote: post #3

Agreed. I mainly manage and review code at this point in my career. I find many bugs, every once in a while finding something that would have caused an outage or notable problem for users. What I find more though is code that isn't thought through. Tech debt and code smell are real, and they affect the performance of a team. Nipping that in the bud takes quality PR reviews and time to meet with submitters around issu…

I am a new manager and I am struggling to get my team to understand the value in code reviews. I have been through so many rewrites and re-re-writes of spaghetti code, I am much more critical now reviewing code, and I am trying to promote this culture on my team. Do you have any suggestions? - The same people leave detailed comments on others' merge requests, but get discouraged when nobody else puts in the same amou…

Good PR culture is definitely something that has to be built from the ground up, and supported top down. At Shopify, who I think has a really good PR culture we have a few things that I think help (beyond a good CICD, and static analysis tools):

1. PRs are supposed to wait for 2 acceptances, can be shipped with 1, and can be emergency shipped with 0. So the barrier is low, but the culture supports more. We are expected to get 2 reviewers from our team to okay.

2. Depending on the code project, we have to fill out a template for the PR, what is in it, what it changes, what to look for when we test the code, etc.

3. Some areas have code owners that might require an additional review from a specific team.

4. We are expected to check out, and test branches when we review them. So a quick read and LGTM is really discouraged outside of a few small cases.

I have seen a lot of places that do the blind PR acceptance, and its tough because without this really being enforced and encouraged that culture is hard to change.

Re: Code reviews do find bugs

#28

Post by alex_lav »

spankalee wrote: post #23
alex_lav wrote: post #20

Earlier quoted context omitted.

You're making a lot of positive-upsided assertions about code review. My point is there is too much opportunity for negative behavior. It's the same as everything in tech, in life, "It can be good if everyone does their part to keep it good". And yet, most don't.

"most don't" is a strong claim. In my experience, core review has been undoubtedly good. I would never run or join a company without it. I'm writing code solo for the moment, and code review is maybe the thing I miss the most.

> "most don't" is a strong claim.

I'm happy to be reasonable. I guess my greater feeling is that most devs aren't great at identifying when they should identify restraint. For the same reason that most devs are abysmal interviewers, I think devs forget that code review is ultimately a human endeavor. Give your average dev the smallest amount of power and not enough guardrails and legitimate silliness ensues.

> I'm writing code solo for the moment, and code review is maybe the thing I miss the most.

I feel as though "code review" is taking on too many meanings in this conversation. Code review in the form of a second (or more) qualified dev reading and commenting on code for the greater good? Obvious good. Code review in the form of github PRs at a non-FAANG company? Skip it. Kangaroo court.

Re: Code reviews do find bugs

#30

Post by 29athrowaway »

alex_lav wrote: post #10

Code reviews can find bugs. More often, code reviews become opportunities for team members to bikeshed. Worse, an opportunity for a non-team member to exert power over a project.

Code reviews can be an opportunity for sabotaging performance or for dominance. I have seen it numerous times.

Conspiracies to delay code reviews for high performers in stacked ranking organizations is common.

They can also be ruined by having the not rotating the reviewer role among eligible reviewers in the team, in that case, everything just represents the opinion of a specific group.

Post reply on HN