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

91–100 of 169 posts

Re: Code reviews do find bugs

#91

Post by majikandy »

jiggawatts wrote: post #80
mikepurvis wrote: post #31

Earlier quoted context omitted.

"Tech debt and code smell are real" I think what I struggle most with is that often times there's a valid business reason to "just ship it ASAP", but the missing piece is the accountability around the conditions attached to that. Like, okay, if we don't want to fix this now because it needs to be in the next release then we can merge it as-is, but you can't document this externally, it can't because part of an API, a…

"There is no later." is my new mantra.

That reminds of codebases littered with Todos… where I like to Yoda it… do or do not, there is no todo.

Re: Code reviews do find bugs

#92

Post by kqr »

poikroequ wrote: post #34

The value of code reviews really depends on the code and the person working on the code. For a team who have spent years working on the same repo, code reviews may not hold much value. But if you have a new guy on the team, or a junior, you'll definitely want to review their code. Code reviews can also do more than just find bugs. You can point out a better way of doing things. Maybe this SQL could be more efficient.…

I got really curious and I'd like to ask you some follow-up questions on your experience in reviewing and receiving reviews. Do you mind shooting an email to hn@xkqr.org?

Re: Code reviews do find bugs

#93

Post by kqr »

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…

This is so far from my experience with code reviews that I'd like to ask some questions to follow up on your experience. Do you mind shooting an email to hn@xkqr.org?

Re: Code reviews do find bugs

#94

Post by topkai22 »

Code reviews don’t just find bugs, they prevent them from being introduced in the first place.

Developers are more careful about what they write and submit when they know they’ll have someone else looking at it.

We went through a couple iterations of our code review policy on a multi-year project a while back. We never really saw code reviews catch a substantial number of bugs over time, but whenever we pulled back on code reviews we definitely saw the production error rate go up.

Re: Code reviews do find bugs

#95

Post by locuscoeruleus »

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…

Talk to people and be curious why they don't value code reviews.

Re: Code reviews do find bugs

#96

Post by kqr »

simonw wrote: post #8
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…

This is certainly true for blocking code reviews. I'm interested in exploring the alternative, which is review-after-commit. There's an article describing those here: https://copyconstruct.medium.com/post-commit-reviews-b4cc216... Code still gets reviewed, but you don't end up with PRs languishing for hours, days or even weeks waiting to get a review from someone.

I really like this idea! It's not like I need to check how any individual developer approaches their work (although that could become a useful mentoring session in some cases) but what matters is what it looks like before going into production.

The main difficulty I see with the described approach is that different changes will be interleaved in the trunk and it might be hard to extract just one of them to deploy. But that's what feature flags are for!

Re: Code reviews do find bugs

#97

Post by majikandy »

azov wrote: post #42
jellyfishbeaver wrote: post #11

Earlier quoted context omitted.

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…

It takes time. For a team not used to code reviews, they might seem more trouble than they're worth at first. Most likely they will be more trouble than they're worth for the first few months. Keep doing them and eventually your smart developers will figure "if we have to do this anyway, we may as well find something useful to say" :) A few things you can do to make it smoother: - Manage expectations. Initially it ma…

This is quite good advice, I feel the “we have to do this anyway” line is more like… “so we might as well make it easy for ourselves”… eg write code that works, you self tested it through tests and manual if needed, so the reviewer doesn’t have to get bogged down in actually running it (start by adding screenshots for this but graduate to not needing them). Keep PRs as small as possible, aka multiple PRs streaming after each other for a single feature card, get the PRs in as soon as valuable and don’t block for nitpics but the shared expectation you start to agree on things that are better and they happen with the next changes.

The general mantra being that “if it works then it shouldn’t be blocked” and developer can choose to improve the maintainability there and then or delay it to next or later PRs at their discretion. After all you trust each other.

Re: Code reviews do find bugs

#98

Post by kqr »

swatcoder wrote: post #51

As with most processes, the dilemma with code reviews is in figuring out how they impact your team and your organization. In a huge org, with thousands of engineers that's already burdened by hours per day of interruptions and process overhead, and release runways that already involve six stamps of bureaucracy, mandatory code revies have very little downside (it's in the noise) but highly variable return (many people…

I don't disagree with the idea that "it depends" but for me, code review has generally worked better with lower overhead in the "startup with five engineers" type organisation. Can I ask you some follow-up questions on your experience in reviewing and receiving reviews? If so, send me an email at hn@xkqr.org!

Re: Code reviews do find bugs

#99

Post by kqr »

rebeccaskinner wrote: post #65

I think the article is taking the wrong view. The statistic cited by the article that 15% of comments were about a bug seems in line with expectations, and I think it would only really be worth discussing if the number were _much higher_ or _much lower_. Instead, I think there are two far more interesting questions to ask: 1. Is the rate at which code review identifies defects sufficient to use code review as a detec…

[flagged]

Re: Code reviews do find bugs

#100

Post by kqr »

bluGill wrote: post #33

Bugs are 'easy' to fix, I don't worry about finding them. I worry about the interfaces as they quickly become a nightmare to change just because of all the users.

I'd be interested to hear more about your experience with code reviews. Could you send an email to hn@xkqr.org so that I can ask some follow-up questions, please?
Post reply on HN