4 ms·

It is a fallacy that it takes a lot of time, like saying QA is worthless because it too takes a lot of time. Just like it's easier to fix a bug when QA finds i

It is a fallacy that it takes a lot of time, like saying QA is worthless because it too takes a lot of time. Just like it's easier to fix a bug when QA finds it before a customer does, it's even easier to fix when a developer spots it in code. If you think you stop making coding mistakes after a few months (well, or ever), you're kidding yourself. I've never got into an argument over code style in a PR, and it seems like if this did happen I'd be looking to fix deeper problems in the team. I've had comments in open source PRs asking me to change something, but that's part of getting into a new code base. The right style is there one that's there, so unless your PR has the sole purpose of specifically changing the style, you should comply with existing style, no arguing. Anyway, there are lots of other pros: A reviewer might suggest a better way of doing something, helping others learn, and likewise will pick up new techniques from the code they're reviewing. This applies to even the most senior, experienced person in the team. If a new library is brought in, it can be checked for conflicting versions or overlap/redundancy with an existing library, and license compliance. Noticing simple things like hardcoded parameters/constants, unchecked config or input values, spots with potential unhanded errors, among dozens of other problems. Unrelated changes are usually caught. Even if it's fixing another bug, I'd argue it's better to do separately with its own issue, because otherwise it gets lost when you go to build release notes or a change log.

Clubber 9y ago
>It is a fallacy that it takes a lot of time, like saying QA is worthless because it too takes a lot of time. If you think it's so valuable, why not have every developer in the team review every merge? That's like 15 code reviews for each piece of code checked in. Obviously, it's because at some point you get diminishing returns. We have developer testing, unit testing, QA and UAT, all to catch defects before production. That's 4 layers of testing. Code reviews make it 5 layers. Not only is it a fifth layer, it's expensive because the most expensive people (in both wages and opportunity cost) are doing it. Reviews are just another iteration of essentially the same thing. At some point, you are spending a lot of money on little return. For me that is mandatory code review after 3-6 months.

Clubber 9y ago

>It is a fallacy that it takes a lot of time, like saying QA is worthless because it too takes a lot of time. If you think it's so valuable, why not have every developer in the team review every merge? That's like 15 code reviews for each piece of code checked in. Obviously, it's because at some point you get diminishing returns. We have developer testing, unit testing, QA and UAT, all to catch defects before production. That's 4 layers of testing. Code reviews make it 5 layers. Not only is it a fifth layer, it's expensive because the most expensive people (in both wages and opportunity cost) are doing it. Reviews are just another iteration of essentially the same thing. At some point, you are spending a lot of money on little return. For me that is mandatory code review after 3-6 months.