Live data from Hacker News

Code reviews do find bugs

two-wrongs.com

81–90 of 169 posts

Re: Code reviews do find bugs

#81

Earlier quoted context omitted.

Sometimes feature requests are actually bugs and can be illustrative of one not properly understanding design. But I think it is important how user feature requests are interpreted. They have a frustration that you might not be aware of but they aren't aware of all the code and constraints. It can even be in design, which is still important. Very often there is a way to resolve a feature request that is not what the…

> Sometimes feature requests are actually bugs You can have two missing features that add up to a bug in total. For example, I worked with two cloud products from the same vendor where a missing back-end HTTP feature of the CDN product interacted with a missing HTTP front-end feature of the PaaS service such that the two products that have a "natural fit" together couldn't actually be used in combination. This made m…

> because not enough people were using the products... because they couldn't.

I don't think this is off topic at all. I think is is explicitly on topic, at least the the underlying one. Not just statistics are hard, but it's hard to measure things and even harder to determine causality. Which is often the underlying goal of statistics and data science. To find out why things happen. Measurements are incredibly difficult and people often think they are simple. The problem is that whatever you're measuring is actually always a proxy and has uncertainty. Often uncertainty you won't know about if you don't have a good understanding of what the metric means. You'll always reap the rewards when putting in the hard work to do this, but unfortunately if you don't it can take time before the seams start to crack. I think this asymmetry is often why people get sloppy.

Re: Code reviews do find bugs

#82
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…

Never realized this was a debated topic. Are there smart people who believe in not having code reviews? What's the best argument against code reviews?

Re: Code reviews do find bugs

#83
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…

>time to meet with submitters around issues you find. What! I would be livid if someone scheduled a meeting with me about a PR. We have way too many meetings already, this is one of the only processes that is mercifully async.

Livid? About a meeting to discuss work? Some comments in slack etc sound worse than intended, and people are aware of that and sometimes go out of their way to say it in a way to it's received as intended.

Re: Code reviews do find bugs

#84
post #44

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…

The previews are valuable though, it makes you look like a wizard when you already know how something broke.

Or incompetent. If you knew this was going to break, why did you approve it? Your only defense is "there was a lot pressure to get into the release xyz". There's not much sympathy for that defense. The animal spirits that thought the broken feature were the most important thing ever are long gone, and frustrations about the new outage caused by the previously most important feature ever have taken over.

Re: Code reviews do find bugs

#85
post #64

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.

[flagged]

Senior team members are usually the worst offenders.

Re: Code reviews do find bugs

#86

Earlier quoted context omitted.

> Sometimes feature requests are actually bugs You can have two missing features that add up to a bug in total. For example, I worked with two cloud products from the same vendor where a missing back-end HTTP feature of the CDN product interacted with a missing HTTP front-end feature of the PaaS service such that the two products that have a "natural fit" together couldn't actually be used in combination. This made m…

> because not enough people were using the products... because they couldn't. I don't think this is off topic at all. I think is is explicitly on topic, at least the the underlying one. Not just statistics are hard, but it's hard to measure things and even harder to determine causality. Which is often the underlying goal of statistics and data science. To find out why things happen. Measurements are incredibly diffic…

The example I like to use is the confusion around COVID statistics, and how people mis-interpreted them.

For example, the rate of infections (or deaths) per day that was reported regularly in the news is actually: rate of infections * measurement accuracy * rate of measurement.

I.e.:

If more people turn up to be tested, the "rate" would go up.

If the PCR tests improved, the "rate" would go up.

A similar thing applies with hospitalisations and deaths. It might go up because a strain is more lethal than another strain, or because more people are infected with the same strain, or because more deaths are attributed to COVID instead of something else.

It doesn't help that different countries have different reporting standards, or that reporting standards changed over time due to the circumstances!

Etc...

It's complicated!

Re: Code reviews do find bugs

#87
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…

>time to meet with submitters around issues you find. What! I would be livid if someone scheduled a meeting with me about a PR. We have way too many meetings already, this is one of the only processes that is mercifully async.

To be clear I make time to meet if the submitter wants to talk through things. I don’t require meeting on every PR. I meet as needed on PRs, pretty infrequently as people get up to speed

Re: Code reviews do find bugs

#89
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…

Never realized this was a debated topic. Are there smart people who believe in not having code reviews? What's the best argument against code reviews?

I’ve spoken with a CTO who was against them because they add too much overhead, partly because they’re too late in the process.

He encouraged his team to discuss an approach beforehand or to work on something together.

Other than that they had a lot of tests and a very structured codebase, I guess it worked for them.

Re: Code reviews do find bugs

#90

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…

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 expect…

Also there is something to be said that code reviews also work well with code that is meant to be reviewed.

The worst kind of peer review happens on PRs that are thousands of lines because nobody wants to read all that and things will be missed. Where I have seen successful code review is where people break code into reviewable bits, and those individual reviews are so fast that they actually end up bring completed faster than if it had been one giant PR.

Post reply on HN