7 ms·

What often goes unmentioned in praise for code review processes is their insanely exorbitant costs -- measured in engineer hours but perhaps more costly is all

What often goes unmentioned in praise for code review processes is their insanely exorbitant costs -- measured in engineer hours but perhaps more costly is all of the blocking and impedance [1]. Most of the "problems" that code reviews claim to address can be solved by much more direct and optimal measures. Code reviews are damn expensive. This post concedes that code reviews are better for the more fluffy ends -- teamwork, openness, social recognition, but given their high costs, I'd rather achieve even these soft goals in other ways than to impede my team's delivery potential. While mission critical systems deserve the whole kitchen sink thrown at them, expensive verifications, code reviews, etc etc., most business applications would do much better optimizing for better software architectures and domain conceptualization than spend so much time dwelling on the minutiae of lines of code. [1] Continuous integration and refactoring, pillars of agility, go out the window in typical code review environments where commits are blocked until peer review.

draw_down 10y ago
Yes, our peer review mostly consists of looking through the diff to make sure nothing seems crazy or out of place, stray print statements, etc. Every single pull request can't go through a gauntlet of close examination, we wouldn't get anything done.
eduren 10y ago
>This post concedes that code reviews are better for the more fluffy ends -- teamwork, openness, social recognition, but given their high costs, I'd rather achieve even these soft goals in other ways than to impede my team's delivery potential. What techniques have you found effective for improving the soft goals in the context of software (genuinely curious)? Are we talking more conventional management/business concepts or something a bit more loosely defined?
- wellpast 10y ago
  Not sure how to formalize but the team's I've worked on have generally negotiated mutual respect, openness, and teamwork by collaborating on the things of greater import -- the architecture, domain conceptualization, etc. And peer code reviews in some cases tend to work against these goals--because you tend to be down in the weeds of LOC, bike-shedding, arguing over the equivalents of tabbing and spacing or whether a line could be more functionally expressed, e.g....
  - thrownaway2424 10y ago
    Well perhaps another part of Google's code review culture is important, then: proposed changes must be style compliant, must compile, must have tests, and all the tests must pass before anyone will bother looking at them. If I got a code review (at Google) that was incorrectly indented, there would be a little red chip in the review UI that indicated such style violation, and I would just reply "Please fix" and not look at it again until the next revision. In the absence of a strong style guide and testing culture then I agree code reviews can get bogged down in bikeshedding. At Google that doesn't seem to happen because the style guide for each language is quite prescriptive. For Go, no change will be reviewed that hasn't passed through gofmt, and so forth. And on the flip side you can have confidence that if your change has gone through gofmt, there's not going to be any discussion about the formatting.
    - alxndr 10y ago
      What UI does Google use for code review?
geophile 10y ago
Amen. Another huge cost of code reviews is distraction. We've all seen the Paul Graham essay on maker's schedules vs. manager's schedules. We've all read the statistics on how much time is lost to interruptions. Code reviews are a massive interruption, done on a manager's schedule. Each code review is a distraction, and can take a significant time commitment, if it is to be a meaningful review. A few years ago, I worked at a company with a strong emphasis on code reviews, and it turned into a waste of time when people couldn't afford to waste time. You'd have to do the review, but you really needed to get back to your development or bug-fixing because of the impending hard deadline, so there would be non-commital non-review reviews such as "looks good to me", or "I see no problems". While a good code review can be valuable, these perfunctory ones are a huge waste of time. Finally, code reviews sometimes substitute for design review, which catches the most serious problems much earlier. At this same company, it drove me crazy that we always had time for code reviews, but never for design reviews.
- devishard 10y ago
  > Another huge cost of code reviews is distraction. We've all seen the Paul Graham essay on maker's schedules vs. manager's schedules. We've all read the statistics on how much time is lost to interruptions. Code reviews are a massive interruption, done on a manager's schedule. Each code review is a distraction, and can take a significant time commitment, if it is to be a meaningful review. Wait why are we doing code reviews on the manager's schedule? Why not just wait until you're finished a task to do them?
  - geophile 10y ago
    What? And keep the other developer waiting?
    - phasmantistes 10y ago
      Code reviews should be asynchronous. Everyone on your team should be able to be working on multiple small changes/commits/whatever in parallel. While one is out for review, they're working on the others. Different people keep different schedules: I do all my reviews first thing in the morning, to settle in, and then often do another round after lunch. Other people do them at the end of the day, or don't mind the interruptions (e.g. follow a pomodoro schedule anyway) and do them as they come in.
CorvusCrypto 10y ago
I feel like this can be argued so many ways. For instance, while code review might block some changes from being introduced into the system, it also does have more benefits than the "fluffy" ones you mentioned. Maybe an engineer that has more experience with the codebase can point out a way of doing something that is more in line with what the team's style is (earlier move toward standardized code vs letting people do what they want). Maybe you missed a utility function that already achieves what some of your code does. Maybe there's not a bug, but again a more experienced dev sees something that can be optimized and will save time in the future that would be spent on tracing system performance issues. The idea that we need to move ultra fast as devs... I think that is necessary in some cases, but I'm not convinced that taking some time to code review tasks is as expensive as you suggested. I mean even reviewing 1000 lines of code should take no more than 30 minutes. And if it looks entirely confusing and hard to follow, then maybe it's something to revisit because I guarantee you're gonna be spending longer if you have to go back and refactor/extend messy and unclean code that technically works. I think the key here is to catch things that stand out and not review the minutae. This is just my opinion of the matter. On the topic of team building, I don't really care for all that stuff. Unit tests w/ CI and periodic refactoring are great for revisiting design decisions. Code reviews are great for catching bad design decisions early on, though so use it as a tool.
- wellpast 10y ago
  So the problem you're talking about is how to properly manage (and coach) less experienced devs or devs who write messy code. While code reviews can tangentially deal with this, I think there are much more direct approaches to these problems than an umbrella code review policy.
  - simplify 10y ago
    What direct approaches do you have in mind?
renku 10y ago
What are these "more direct and optimal measures" that are cheaper than code reviews? In my experience code reviews are much cheaper than other means of raising software quality. For example unit tests only start to add real value after you have written a good bunch of them, so they form this regression-safety-net. I haven't seen Continuous integration and refactoring being thrown out of the window because of code reviews. Quite the opposite. Code reviews usually ask the author to refactor his code further. Similarly I have no idea why one would stop continuous integration because of code reviews.
- wellpast 10y ago
  Continuous integration means you can work on multiple items at once and that you break up large feature dev into smaller chunks that get committed to master. For example, I might commit the data layer for a new big feature well before the UX, etc. Code reviews mean that every time I want to integrate with master I've got to 'grab a lock' and schedule and wait for a code review. What typically happens in these code review cultures is you tend to make monolithic commits (to minimize the # of code reviews you have to do) and you also tend to minimize diffs with the code base (to make the CR easier for the reviewer).
  - phasmantistes 10y ago
    What? These statements and conclusions make no sense to me. Code reviews mean that every time I want to integrate with master I've got to 'grab a lock' and schedule and wait for a code review. You're using a terrible code review system. You should be able to have multiple potential commits out for review at any given time. You should also have multiple pending commits from your coworkers on your queue at any given time. The code review process is asyncrhonous: process them at your leisure (though sooner is better than later, for obvious reasons). Once the review has been approved, it can be rebased onto or merged into master. If the patches you're writing are so far-flung that they don't apply cleanly, break them into smaller pieces. What typically happens in these code review cultures is you tend to make monolithic commits (to minimize the # of code reviews you have to do) and you also tend to minimize diffs with the code base (to make the CR easier for the reviewer). These two statements are contradictory. Monolithic commits by definition increase the diff with the codebase, not decrease it. What you tend to see are meaningful commits: commits which do one specific thing, and fully test it, without breaking any other systems.
kerkeslager 10y ago
> What often goes unmentioned in praise for code review processes is their insanely exorbitant costs -- measured in engineer hours but perhaps more costly is all of the blocking and impedance [1]. Most of the "problems" that code reviews claim to address can be solved by much more direct and optimal measures. Code reviews are damn expensive. I'm on a team where for about a year, part of our process has been that code doesn't get merged until three people say it's ready. If one person wrote it, it needs two reviews, or if a pair wrote it, it needs one review. Your mileage may vary, but my experience with this is that the code reviews increase latency, but don't really have much effect on bandwidth. It might mean that there's a few hours between finishing your code and merging it into master, but during those hours you can be doing other things. Most code reviews take <10 minutes, and I usually just check for other people's PRs when right after I create a PR of my own, so it doesn't interrupt my flow. Sometimes PRs do get blocked, but in those cases it's usually for good reasons; the design has some serious flaw or is very overcomplicated, or there's a bug. I see that as similar to a failing build. Sure, a CI server sometimes blocks you from merging, but do you really want something that fails tests or doesn't compile going into your master branch anyway? Code review is just like having a CI server that builds and runs tests, but it checks things that can't be automated. We do sometimes run into cases where code review takes a long time, but the root cause there is usually that we didn't break down the feature into an adequately small enough minimum viable product. 1000 lines of changes takes more than 10 times as long to review as 100 lines of changes; the increase isn't linear. If code review is taking a long time, it may be an indication that your continuous integration isn't continuous enough. That said, "code review is 100% necessary" and "code review is 100% too costly" are just different kinds of Kool-Aid you shouldn't drink. "People and interactions over processes and tools"[1] applies here. What works for my team might not work for yours. I'm not arguing for code review; there's no universal right way to create software. I'm arguing that code review is an effective process step for some teams and an ineffective one for others. [1] http://www.agilemanifesto.org/ http://www.agilemanifesto.org/
- r-s 10y ago
  I have a hard time imagining that most code reviews take < 10 minutes. * Understanding of the problem at hand, reading through the story, understanding the context of the the code change. * Pulling down the code, reading through the commit log * Reading the specs, possibly running coverage tools if not part of testing suite. * Understanding the logic of the code, seeing if the tests cover the edge cases * If dependencies change, they may need to be investigated too * Thinking if there are better ways make code a bit more extensible or understandable * Thinking of risk of the code (security, performance, deployment concerns) * Reading through associated documentation and ensuring its accuracy Doing all of this takes me longer then 10 minutes always.
ArkyBeagle 10y ago
1) Commits don't have to be blocked. The action items from a review can be a separate commit. 2) Reviews can be done privately, with comments entered electronically. There doesn't have to be a four-day meeting. 2a) Organizations that use four-day meetings are likely full of people who need something to charge that time to. It may be worth delegating a representative from the engineering team to attend those. 3) Action items must be triaged. Ideally, each action item would be assigned a number and numbers grouped together as a statement of work for a commit. This is much less painful than it sounds. Test provenance would also be nice per commit.
jupp0r 10y ago
If anyone is ever blocked, you are doing it wrong. When performed on git branches, you can easily branch off you current feature branch, keep going and rebase to master when the original branch has been merged. Continuous integration is with you all the time and helps the reviewer by annotating the diff with code coverage, cyclomatic complexity and other metrics. A good review culture emphasizes design feedback over low level issues. In addition, plenty of positive feedback is as important as finding issues.
nvarsj 10y ago
> What often goes unmentioned in praise for code review processes is their insanely exorbitant costs -- measured in engineer hours but perhaps more costly is all of the blocking and impedance [1]. I don't get why you think code reviews are so expensive. An engineer should rarely be blocked by a code review. If waiting for a review - don't. Pick up another task! A great thing about code reviews is they are done async. If the cost is time spent doing the review - what's the alternative? No code review at all? That's like not testing - yes it's faster now, but you'll pay for it dearly later on. (Code review is well studied in academia / industry, and has consistently shown to be very effective. Having at least one extra pair of eyeballs on any code goes a long way.)
- geophile 10y ago
  As the comments all over this thread demonstrate, code review is done for many reasons. Where I've worked, the reasons have not been clearly stated, and as a result there is a lot of time wasted. I wouldn't say that there should not be code reviews, but they could be cut way down. A code review should not be a substitute for a design review. It often is, in my experience. A code review should not be done for anything that can be done by static code analysis: code style, finding dead code, etc. Also, a lot depends on how code reviews are done. I've seen them used as a veto, with lots of back and forth to get the veto removed over subjective and trivial issues. Don't do that. Finally, let's be honest. Some people's code doesn't need review. Time spent reviewing such code is wasted. But it is often socially difficult to say this, so everyone's code gets reviewed. Other people can't be trusted to write acceptable code, and reviews are essential. That's a different problem. (Beyond the scope of this note. Left as an exercise to the reader. Other cliches may apply.)
knorker 10y ago
> insanely exorbitant costs wat? Either the code is easily readable and correct, and takes barely any time at all to review, or it's buggy and detecting this early rather than late saves a yuuuuge amount of time. Also, the reviewer now knows the code. So in reviewing the code they've already half way to being able to improve the code in the future. And the reviewer can also pick up "oh, that's a neat pattern". Code reviews are education for both parties. Are you saying companies should not spend time or money on education/courses? And as an author I really appreciate not only that someone looks for me having made a mistake, but because I know someone will read the code I won't go with the easy way. I can fool myself, but I can't fool someone else. E.g. the right thing to do is to name this constant or make an enum, but I'll just put a literal "4" here because I know it's 4. (which of course I won't remember in a week).

draw_down 10y ago

Yes, our peer review mostly consists of looking through the diff to make sure nothing seems crazy or out of place, stray print statements, etc. Every single pull request can't go through a gauntlet of close examination, we wouldn't get anything done.

eduren 10y ago

>This post concedes that code reviews are better for the more fluffy ends -- teamwork, openness, social recognition, but given their high costs, I'd rather achieve even these soft goals in other ways than to impede my team's delivery potential. What techniques have you found effective for improving the soft goals in the context of software (genuinely curious)? Are we talking more conventional management/business concepts or something a bit more loosely defined?

wellpast 10y ago
Not sure how to formalize but the team's I've worked on have generally negotiated mutual respect, openness, and teamwork by collaborating on the things of greater import -- the architecture, domain conceptualization, etc. And peer code reviews in some cases tend to work against these goals--because you tend to be down in the weeds of LOC, bike-shedding, arguing over the equivalents of tabbing and spacing or whether a line could be more functionally expressed, e.g....
- thrownaway2424 10y ago
  Well perhaps another part of Google's code review culture is important, then: proposed changes must be style compliant, must compile, must have tests, and all the tests must pass before anyone will bother looking at them. If I got a code review (at Google) that was incorrectly indented, there would be a little red chip in the review UI that indicated such style violation, and I would just reply "Please fix" and not look at it again until the next revision. In the absence of a strong style guide and testing culture then I agree code reviews can get bogged down in bikeshedding. At Google that doesn't seem to happen because the style guide for each language is quite prescriptive. For Go, no change will be reviewed that hasn't passed through gofmt, and so forth. And on the flip side you can have confidence that if your change has gone through gofmt, there's not going to be any discussion about the formatting.
  - alxndr 10y ago
    What UI does Google use for code review?

geophile 10y ago

Amen. Another huge cost of code reviews is distraction. We've all seen the Paul Graham essay on maker's schedules vs. manager's schedules. We've all read the statistics on how much time is lost to interruptions. Code reviews are a massive interruption, done on a manager's schedule. Each code review is a distraction, and can take a significant time commitment, if it is to be a meaningful review. A few years ago, I worked at a company with a strong emphasis on code reviews, and it turned into a waste of time when people couldn't afford to waste time. You'd have to do the review, but you really needed to get back to your development or bug-fixing because of the impending hard deadline, so there would be non-commital non-review reviews such as "looks good to me", or "I see no problems". While a good code review can be valuable, these perfunctory ones are a huge waste of time. Finally, code reviews sometimes substitute for design review, which catches the most serious problems much earlier. At this same company, it drove me crazy that we always had time for code reviews, but never for design reviews.

devishard 10y ago
> Another huge cost of code reviews is distraction. We've all seen the Paul Graham essay on maker's schedules vs. manager's schedules. We've all read the statistics on how much time is lost to interruptions. Code reviews are a massive interruption, done on a manager's schedule. Each code review is a distraction, and can take a significant time commitment, if it is to be a meaningful review. Wait why are we doing code reviews on the manager's schedule? Why not just wait until you're finished a task to do them?
- geophile 10y ago
  What? And keep the other developer waiting?
  - phasmantistes 10y ago
    Code reviews should be asynchronous. Everyone on your team should be able to be working on multiple small changes/commits/whatever in parallel. While one is out for review, they're working on the others. Different people keep different schedules: I do all my reviews first thing in the morning, to settle in, and then often do another round after lunch. Other people do them at the end of the day, or don't mind the interruptions (e.g. follow a pomodoro schedule anyway) and do them as they come in.

CorvusCrypto 10y ago

I feel like this can be argued so many ways. For instance, while code review might block some changes from being introduced into the system, it also does have more benefits than the "fluffy" ones you mentioned. Maybe an engineer that has more experience with the codebase can point out a way of doing something that is more in line with what the team's style is (earlier move toward standardized code vs letting people do what they want). Maybe you missed a utility function that already achieves what some of your code does. Maybe there's not a bug, but again a more experienced dev sees something that can be optimized and will save time in the future that would be spent on tracing system performance issues. The idea that we need to move ultra fast as devs... I think that is necessary in some cases, but I'm not convinced that taking some time to code review tasks is as expensive as you suggested. I mean even reviewing 1000 lines of code should take no more than 30 minutes. And if it looks entirely confusing and hard to follow, then maybe it's something to revisit because I guarantee you're gonna be spending longer if you have to go back and refactor/extend messy and unclean code that technically works. I think the key here is to catch things that stand out and not review the minutae. This is just my opinion of the matter. On the topic of team building, I don't really care for all that stuff. Unit tests w/ CI and periodic refactoring are great for revisiting design decisions. Code reviews are great for catching bad design decisions early on, though so use it as a tool.

wellpast 10y ago
So the problem you're talking about is how to properly manage (and coach) less experienced devs or devs who write messy code. While code reviews can tangentially deal with this, I think there are much more direct approaches to these problems than an umbrella code review policy.
- simplify 10y ago
  What direct approaches do you have in mind?

renku 10y ago

What are these "more direct and optimal measures" that are cheaper than code reviews? In my experience code reviews are much cheaper than other means of raising software quality. For example unit tests only start to add real value after you have written a good bunch of them, so they form this regression-safety-net. I haven't seen Continuous integration and refactoring being thrown out of the window because of code reviews. Quite the opposite. Code reviews usually ask the author to refactor his code further. Similarly I have no idea why one would stop continuous integration because of code reviews.

wellpast 10y ago
Continuous integration means you can work on multiple items at once and that you break up large feature dev into smaller chunks that get committed to master. For example, I might commit the data layer for a new big feature well before the UX, etc. Code reviews mean that every time I want to integrate with master I've got to 'grab a lock' and schedule and wait for a code review. What typically happens in these code review cultures is you tend to make monolithic commits (to minimize the # of code reviews you have to do) and you also tend to minimize diffs with the code base (to make the CR easier for the reviewer).
- phasmantistes 10y ago
  What? These statements and conclusions make no sense to me. Code reviews mean that every time I want to integrate with master I've got to 'grab a lock' and schedule and wait for a code review. You're using a terrible code review system. You should be able to have multiple potential commits out for review at any given time. You should also have multiple pending commits from your coworkers on your queue at any given time. The code review process is asyncrhonous: process them at your leisure (though sooner is better than later, for obvious reasons). Once the review has been approved, it can be rebased onto or merged into master. If the patches you're writing are so far-flung that they don't apply cleanly, break them into smaller pieces. What typically happens in these code review cultures is you tend to make monolithic commits (to minimize the # of code reviews you have to do) and you also tend to minimize diffs with the code base (to make the CR easier for the reviewer). These two statements are contradictory. Monolithic commits by definition increase the diff with the codebase, not decrease it. What you tend to see are meaningful commits: commits which do one specific thing, and fully test it, without breaking any other systems.

kerkeslager 10y ago

> What often goes unmentioned in praise for code review processes is their insanely exorbitant costs -- measured in engineer hours but perhaps more costly is all of the blocking and impedance [1]. Most of the "problems" that code reviews claim to address can be solved by much more direct and optimal measures. Code reviews are damn expensive. I'm on a team where for about a year, part of our process has been that code doesn't get merged until three people say it's ready. If one person wrote it, it needs two reviews, or if a pair wrote it, it needs one review. Your mileage may vary, but my experience with this is that the code reviews increase latency, but don't really have much effect on bandwidth. It might mean that there's a few hours between finishing your code and merging it into master, but during those hours you can be doing other things. Most code reviews take <10 minutes, and I usually just check for other people's PRs when right after I create a PR of my own, so it doesn't interrupt my flow. Sometimes PRs do get blocked, but in those cases it's usually for good reasons; the design has some serious flaw or is very overcomplicated, or there's a bug. I see that as similar to a failing build. Sure, a CI server sometimes blocks you from merging, but do you really want something that fails tests or doesn't compile going into your master branch anyway? Code review is just like having a CI server that builds and runs tests, but it checks things that can't be automated. We do sometimes run into cases where code review takes a long time, but the root cause there is usually that we didn't break down the feature into an adequately small enough minimum viable product. 1000 lines of changes takes more than 10 times as long to review as 100 lines of changes; the increase isn't linear. If code review is taking a long time, it may be an indication that your continuous integration isn't continuous enough. That said, "code review is 100% necessary" and "code review is 100% too costly" are just different kinds of Kool-Aid you shouldn't drink. "People and interactions over processes and tools"[1] applies here. What works for my team might not work for yours. I'm not arguing for code review; there's no universal right way to create software. I'm arguing that code review is an effective process step for some teams and an ineffective one for others. [1] http://www.agilemanifesto.org/ http://www.agilemanifesto.org/

r-s 10y ago
I have a hard time imagining that most code reviews take < 10 minutes. * Understanding of the problem at hand, reading through the story, understanding the context of the the code change. * Pulling down the code, reading through the commit log * Reading the specs, possibly running coverage tools if not part of testing suite. * Understanding the logic of the code, seeing if the tests cover the edge cases * If dependencies change, they may need to be investigated too * Thinking if there are better ways make code a bit more extensible or understandable * Thinking of risk of the code (security, performance, deployment concerns) * Reading through associated documentation and ensuring its accuracy Doing all of this takes me longer then 10 minutes always.

ArkyBeagle 10y ago

1) Commits don't have to be blocked. The action items from a review can be a separate commit. 2) Reviews can be done privately, with comments entered electronically. There doesn't have to be a four-day meeting. 2a) Organizations that use four-day meetings are likely full of people who need something to charge that time to. It may be worth delegating a representative from the engineering team to attend those. 3) Action items must be triaged. Ideally, each action item would be assigned a number and numbers grouped together as a statement of work for a commit. This is much less painful than it sounds. Test provenance would also be nice per commit.

jupp0r 10y ago

If anyone is ever blocked, you are doing it wrong. When performed on git branches, you can easily branch off you current feature branch, keep going and rebase to master when the original branch has been merged. Continuous integration is with you all the time and helps the reviewer by annotating the diff with code coverage, cyclomatic complexity and other metrics. A good review culture emphasizes design feedback over low level issues. In addition, plenty of positive feedback is as important as finding issues.

nvarsj 10y ago

> What often goes unmentioned in praise for code review processes is their insanely exorbitant costs -- measured in engineer hours but perhaps more costly is all of the blocking and impedance [1]. I don't get why you think code reviews are so expensive. An engineer should rarely be blocked by a code review. If waiting for a review - don't. Pick up another task! A great thing about code reviews is they are done async. If the cost is time spent doing the review - what's the alternative? No code review at all? That's like not testing - yes it's faster now, but you'll pay for it dearly later on. (Code review is well studied in academia / industry, and has consistently shown to be very effective. Having at least one extra pair of eyeballs on any code goes a long way.)

geophile 10y ago
As the comments all over this thread demonstrate, code review is done for many reasons. Where I've worked, the reasons have not been clearly stated, and as a result there is a lot of time wasted. I wouldn't say that there should not be code reviews, but they could be cut way down. A code review should not be a substitute for a design review. It often is, in my experience. A code review should not be done for anything that can be done by static code analysis: code style, finding dead code, etc. Also, a lot depends on how code reviews are done. I've seen them used as a veto, with lots of back and forth to get the veto removed over subjective and trivial issues. Don't do that. Finally, let's be honest. Some people's code doesn't need review. Time spent reviewing such code is wasted. But it is often socially difficult to say this, so everyone's code gets reviewed. Other people can't be trusted to write acceptable code, and reviews are essential. That's a different problem. (Beyond the scope of this note. Left as an exercise to the reader. Other cliches may apply.)

knorker 10y ago

> insanely exorbitant costs wat? Either the code is easily readable and correct, and takes barely any time at all to review, or it's buggy and detecting this early rather than late saves a yuuuuge amount of time. Also, the reviewer now knows the code. So in reviewing the code they've already half way to being able to improve the code in the future. And the reviewer can also pick up "oh, that's a neat pattern". Code reviews are education for both parties. Are you saying companies should not spend time or money on education/courses? And as an author I really appreciate not only that someone looks for me having made a mistake, but because I know someone will read the code I won't go with the easy way. I can fool myself, but I can't fool someone else. E.g. the right thing to do is to name this constant or make an enum, but I'll just put a literal "4" here because I know it's 4. (which of course I won't remember in a week).