3 ms·

in real companies that don't have G's resources, quality and velocity are tradeoffs, not things you can have both of god bless them for trying though in parti

in real companies that don't have G's resources, quality and velocity are tradeoffs, not things you can have both of god bless them for trying though in particular, the idea of having one or more reviewers doing multiple rounds in a day is pure fiction in strapped startups. Rejecting a code review for readability isn't always an option. Even getting a timely review from someone who understands this part of the codebase can be unrealistic -- sometimes there is no such person. Sometimes nobody is left in the company who understands the affected module and even the PR author is taking confession before merging it. Love the phrase 'unless it's an emergency'. In resource-constrained cos everything is an emergency because to get any non-emergency work done requires emergency level urgency. G's way of life requires an even power balance between eng & other silos. And also requires a baseline level of technical involvement by PMs which isn't present outside big tech cos. Most companies just aren't hiring PMs with a programming background. They could never justify the spend. Major missing section in this is 'what shouldn't you review' -- various kinds of nits and out-of-scope changes that derail the process, waste expensive CI cycles, and make people take the rest of the day off from frustration.

BigJono 7y ago
It's not supposed to be a one size fits all I guess. For example, their recommended order of reviewing code tests first doesn't exactly work when you have no tests, as is pretty common at startups.
iamchrisle 7y ago
I think the saying is "Quality, speed, cost. Pick two?"
tylerl 7y ago
Having experienced in real life the benefits of this policy myself, I absolutely disagree. I would institute Google-style code reviews at any company I went to that was large enough to have teams of 3 people or more. This as much for efficiency as for quality. The development pace isn't set by the speed at which new lines of code get added, is the speed at which features can reach maturity, and in general more extensive code reviews can speed things up in the aggregate in addition to making things work better. At Google, every single line of code has to be signed off by at least one other engineer _before_ it can be committed to the official codebase, regardless of team size. Additionally, unit tests are required, typically at the same code gets reviewed. This all makes code submission significantly slower, but product development significantly faster. Why? Here's a few reasons I've personally observed: 1: Bus factor. People leave the team all the time, ideally not for the reason the concept is named after. Having someone thoroughly review your code means there are at least 2 people on the team who completely understand every single line of code. That guarantee isn't absolute just because of timing and churn, but it's correct often enough to be the rule. This simple fact has saved me, personally, _days_ worth of stumbling around. It saves you from precisely the failure condition you described. 2: Fewer changes. Especially paired with the "small CLs" advice, this helps you really quickly correct course when people are headed down the wrong solution. As a result, you end up shipping a much higher percentage of the code that you actually write. 3: Fewer bugs. This is the biggest item of them all. Bugs are the quicksand of coding... the metric you hear quoted is that companies are expected to waste 75% of their development time on debugging. Reviewers easily catch enough mistakes to more than pay for their time investment right there. This is especially true for misfeatures where the code is "correct", it just correctly does the wrong thing. Those mistakes are hard to catch with tests (because the tests will be equally wrong), but easy to catch with humans. Also, this is where a zero-tolerance policy on requiring unit tests really shines. I've been at this for many years, and I have had zero (zero!) bugs in the unit-tested code that I've committed. My tests have found some that would have been real awful brain-melters though. I'm on slow end of things; it takes me about 5x as long to write the tests for a given module than to write the module itself, and I'm still WAY ahead on time when I account my own debugging savings (I think the 75% number is low if you want to hit the zero bug mark). On my (30-person) team, we spend about 5% to 10% of our time dealing with bugs in our own code, and regularly hit the "feature requests only" mark in our queue. And the bugs we do get we can generally eliminate as a class by teaching better review and test practices.
Cthulhu_ 7y ago
And yet. Technical debt costs you time and money over time. Fixing code six months down the line takes 5-10x as much effort than just fixing it while it's still fresh. Worst case scenarios is where you end up throwing away an application and rebuild it because the technical debt has made progress grind to a halt. If there's nobody left who understands the code, or if there's hero coders who 'own' one section of the application, it's a huge risk to the company - which can, and has, bring a whole company down (there's a few stories on the daily wtf iirc). A recurring thought with me and my projects after about six months is always "I wish we had done this properly the first time around"; in hindsight there was more than enough time, and it's costing a lot more time to fix it now.
- awinter-py 7y ago
  when a group of people decide together what 'properly' will look like in hindsight in 6 mos, you can end up with some interesting outcomes

BigJono 7y ago

It's not supposed to be a one size fits all I guess. For example, their recommended order of reviewing code tests first doesn't exactly work when you have no tests, as is pretty common at startups.

iamchrisle 7y ago

I think the saying is "Quality, speed, cost. Pick two?"

tylerl 7y ago

Having experienced in real life the benefits of this policy myself, I absolutely disagree. I would institute Google-style code reviews at any company I went to that was large enough to have teams of 3 people or more. This as much for efficiency as for quality. The development pace isn't set by the speed at which new lines of code get added, is the speed at which features can reach maturity, and in general more extensive code reviews can speed things up in the aggregate in addition to making things work better. At Google, every single line of code has to be signed off by at least one other engineer _before_ it can be committed to the official codebase, regardless of team size. Additionally, unit tests are required, typically at the same code gets reviewed. This all makes code submission significantly slower, but product development significantly faster. Why? Here's a few reasons I've personally observed: 1: Bus factor. People leave the team all the time, ideally not for the reason the concept is named after. Having someone thoroughly review your code means there are at least 2 people on the team who completely understand every single line of code. That guarantee isn't absolute just because of timing and churn, but it's correct often enough to be the rule. This simple fact has saved me, personally, _days_ worth of stumbling around. It saves you from precisely the failure condition you described. 2: Fewer changes. Especially paired with the "small CLs" advice, this helps you really quickly correct course when people are headed down the wrong solution. As a result, you end up shipping a much higher percentage of the code that you actually write. 3: Fewer bugs. This is the biggest item of them all. Bugs are the quicksand of coding... the metric you hear quoted is that companies are expected to waste 75% of their development time on debugging. Reviewers easily catch enough mistakes to more than pay for their time investment right there. This is especially true for misfeatures where the code is "correct", it just correctly does the wrong thing. Those mistakes are hard to catch with tests (because the tests will be equally wrong), but easy to catch with humans. Also, this is where a zero-tolerance policy on requiring unit tests really shines. I've been at this for many years, and I have had zero (zero!) bugs in the unit-tested code that I've committed. My tests have found some that would have been real awful brain-melters though. I'm on slow end of things; it takes me about 5x as long to write the tests for a given module than to write the module itself, and I'm still WAY ahead on time when I account my own debugging savings (I think the 75% number is low if you want to hit the zero bug mark). On my (30-person) team, we spend about 5% to 10% of our time dealing with bugs in our own code, and regularly hit the "feature requests only" mark in our queue. And the bugs we do get we can generally eliminate as a class by teaching better review and test practices.

Cthulhu_ 7y ago

And yet. Technical debt costs you time and money over time. Fixing code six months down the line takes 5-10x as much effort than just fixing it while it's still fresh. Worst case scenarios is where you end up throwing away an application and rebuild it because the technical debt has made progress grind to a halt. If there's nobody left who understands the code, or if there's hero coders who 'own' one section of the application, it's a huge risk to the company - which can, and has, bring a whole company down (there's a few stories on the daily wtf iirc). A recurring thought with me and my projects after about six months is always "I wish we had done this properly the first time around"; in hindsight there was more than enough time, and it's costing a lot more time to fix it now.

awinter-py 7y ago
when a group of people decide together what 'properly' will look like in hindsight in 6 mos, you can end up with some interesting outcomes