5 ms·

While I cannot judge how those practices work at Google, I have seen similar practices fail miserably when applied in smaller companies. Opinionated best pract

by hibikir 12y ago

While I cannot judge how those practices work at Google, I have seen similar practices fail miserably when applied in smaller companies. Opinionated best practices stop being useful when the emphasis is on opinionated and not on best. If the cabal that has political influence is not the strongest there is, standardization just increases how much technical debt can be built in a short period of time. I have seen expenses on tools teams that went nowhere, for two reasons. The first is that tools teams are attractive to people that do not want to deal with customers, so they can attract the opinionated and antisocial. The other problem is that the generic tools really have to solve the problem very well: Lack the engineering quality in said tools team, and you are spending a bunch of money on infrastructure that will both fail to evolve at the speed of OSS, and get entrenched everywhere in your codebase. Automated testing will only get you far if it is pragmatic. I have seen millions of dollars spent on test automation that was extremely fragile and never paid for itself, because it had an extremely short shelf life. Picking the wrong tools for it makes it even worse. Code reviews can be great, or can be terrible. A code review searching for real problems in the code, or that really considers alternative solutions, will be very valuable. But code reviews can quickly become pissing contests used by people to impose their preferences on others: A passive aggressive way of asserting dominance on a team. You can see a lot of that in proponents of the 5 minute code review: No actual analysis of the code is done, but 5 minutes is plenty to use it as a tool for abuse. So while the techniques described in the article can be very helpful, the wrong implementation of them will just help you standardize on a monoculture of bad engineering. So before implementing such things think about how good your engineering really is, and whether you really are better off making sure people bend to your standards, or you are better off learning from the experience of new people. Chances are you are not Google.

mreiland 12y ago
That's pretty much always been my opinion on code reviews. They're too often wielded as a bullying tactic.
- jdotjdot 12y ago
  Have to put out a shameless plug here for Hacker School. I looked forward to code reviews there because I learned so much from them. This is to the point that it actually surprised me reading your comment about how code reviews have been unpleasant for you. Would be great if more workplaces could adopt that kind of mindset and set of values as well. In short, code reviews don't have to be bad, from personal experience.
  - mreiland 12y ago
    2 things 1. I never said I've had unpleasant experiences with code reviews, I said they're too often wielded as a bullying tactic. This is something that can be recognized without actually being a part of the bullying tactic. Furthermore, your comment attempts to subvert my statement by implying my observation is more about bad personal experiences and less about thoughtfulness. 2. The context for which code reviews has been used is not throwing up some code on a random website and asking random strangers to look over it for educational purposes. The context is one of a professional environment where code is typically required to be reviewed by another professional before being allowed into the general codebase in an attempt to both educate about the current codebase, catch possible inefficiencies, and to try and maintain some form of quality control with respect to the mechanical aspects of the code. These are completely different contexts. On a site like Hacker School the worst you're going to get is people mouthing off about shit. In a professional environment, the worst you'll get is fired. Very different environments.
    - jdotjdot 12y ago
      Hacker School is not a random website where you throw up code to have random strangers look at it. It is a three-month-long "writer's retreat for programmers," where you are sitting and working with other developers every single day, just like in a workplace, except that you're working on what you like and won't be fired. Code reviews are in person and could easily become unprofessional or attacking--but they don't because there are very strong social rules about how to conduct them effectively and with improvement in mind rather than criticism. I respect the point about thoughtfulness vs personal experience, but you should at least make the effort to look up what Hacker School is before making comments about it.
- bowlofpetunias 12y ago
  In a toxic environment, every tool and process is abused.
  - mreiland 12y ago
    It doesn't take a toxic environment, only a single person wanting to be a bully.
contingencies 12y ago
I have seen similar practices fail miserably when applied in smaller companies. Me too. Opinionated best practices stop being useful when the emphasis is on opinionated and not on best. Sure. Also, if changing the way things are done causes friction for the team, sometimes it's not worth it. There's always tradeoffs. Lack the engineering quality in said tools team, and you are spending a bunch of money on infrastructure that will both fail to evolve at the speed of OSS, and get entrenched everywhere in your codebase. This is a great argument for investing in OSS tools, and for open sourcing tools invented in-house. Automated testing will only get you far if it is pragmatic. I have seen millions of dollars spent on test automation that was extremely fragile and never paid for itself, because it had an extremely short shelf life. Picking the wrong tools for it makes it even worse. When discussing the relative pragmatism of tests, it can be important to understand where they live within the perceived development workflow. Generally, it makes sense to attach fast, automated tests to precommit hooks in your version control system. Slow, automated tests can be postcommit and run on shared infrastructure. Very slow, exhaustive regression tests involving repeated, complex, time-consuming virtual infrastructure setup can be run when committing new tags/releases. It makes little sense for regular developers to focus on the latter two. However, the former is a great target for extra tests that's generally approachable by and visible to all. But code reviews can quickly become pissing contests used by people to impose their preferences on others: A passive aggressive way of asserting dominance on a team. You can see a lot of that in proponents of the 5 minute code review: No actual analysis of the code is done, but 5 minutes is plenty to use it as a tool for abuse. To avoid these issues it can be better to do these written/offline than in person. Maybe then discuss the results in person. So while the techniques described in the article can be very helpful, the wrong implementation of them will just help you standardize on a monoculture of bad engineering. So before implementing such things think about how good your engineering really is, and whether you really are better off making sure people bend to your standards, or you are better off learning from the experience of new people. Chances are you are not Google. Very well said.
jrockway 12y ago
You have to be careful about who you hire. If you do that well, then opinions, code reviews, one shared repository, and so on, work great. If you hire a bunch of assholes, then they won't work. I do about 20 code reviews a week; there is never bullying and they are only rarely rubber stamped. (My favorite code review of my own code this week was where I updated a counter to use atomic.AddInt64 instead of sync.Mutex. But where I was reading the counter, I just read the integer value directly instead of using atomic.LoadInt64. 30 seconds of the reviewers time, and we avoided a potentially difficult-to-track-down bug. No pissing match, no bullying. Just better programs.)
- watwut 12y ago
  Any company over 30 people will have at least one borderline asshole and one asshole is enough to poison the code review if managed wrong.
- kabouseng 12y ago
  20 code reviews a week? How many lines of code for each review?
  - jrockway 12y ago
    I looked in more detail and if I average it out over 2.5 years and only include actual code I care about (not importing OSS packages, which I also review), it's more like one a day. I was too lazy to calculate a histogram, but let's go with ~100 lines a day.
    - kabouseng 12y ago
      Thanks for the reply.
EdwardDiego 12y ago
> Opinionated best practices stop being useful when the emphasis is on opinionated and not on best. If the cabal that has political influence is not the strongest there is, standardization just increases how much technical debt can be built in a short period of time. I agree so, so, so very much. We've had issues where one teams wants everyone to standardise on a given technology for ideological rather than practical reasons.

by hibikir 12y ago

mreiland 12y ago

That's pretty much always been my opinion on code reviews. They're too often wielded as a bullying tactic.

jdotjdot 12y ago
Have to put out a shameless plug here for Hacker School. I looked forward to code reviews there because I learned so much from them. This is to the point that it actually surprised me reading your comment about how code reviews have been unpleasant for you. Would be great if more workplaces could adopt that kind of mindset and set of values as well. In short, code reviews don't have to be bad, from personal experience.
- mreiland 12y ago
  2 things 1. I never said I've had unpleasant experiences with code reviews, I said they're too often wielded as a bullying tactic. This is something that can be recognized without actually being a part of the bullying tactic. Furthermore, your comment attempts to subvert my statement by implying my observation is more about bad personal experiences and less about thoughtfulness. 2. The context for which code reviews has been used is not throwing up some code on a random website and asking random strangers to look over it for educational purposes. The context is one of a professional environment where code is typically required to be reviewed by another professional before being allowed into the general codebase in an attempt to both educate about the current codebase, catch possible inefficiencies, and to try and maintain some form of quality control with respect to the mechanical aspects of the code. These are completely different contexts. On a site like Hacker School the worst you're going to get is people mouthing off about shit. In a professional environment, the worst you'll get is fired. Very different environments.
  - jdotjdot 12y ago
    Hacker School is not a random website where you throw up code to have random strangers look at it. It is a three-month-long "writer's retreat for programmers," where you are sitting and working with other developers every single day, just like in a workplace, except that you're working on what you like and won't be fired. Code reviews are in person and could easily become unprofessional or attacking--but they don't because there are very strong social rules about how to conduct them effectively and with improvement in mind rather than criticism. I respect the point about thoughtfulness vs personal experience, but you should at least make the effort to look up what Hacker School is before making comments about it.
bowlofpetunias 12y ago
In a toxic environment, every tool and process is abused.
- mreiland 12y ago
  It doesn't take a toxic environment, only a single person wanting to be a bully.

contingencies 12y ago

I have seen similar practices fail miserably when applied in smaller companies. Me too. Opinionated best practices stop being useful when the emphasis is on opinionated and not on best. Sure. Also, if changing the way things are done causes friction for the team, sometimes it's not worth it. There's always tradeoffs. Lack the engineering quality in said tools team, and you are spending a bunch of money on infrastructure that will both fail to evolve at the speed of OSS, and get entrenched everywhere in your codebase. This is a great argument for investing in OSS tools, and for open sourcing tools invented in-house. Automated testing will only get you far if it is pragmatic. I have seen millions of dollars spent on test automation that was extremely fragile and never paid for itself, because it had an extremely short shelf life. Picking the wrong tools for it makes it even worse. When discussing the relative pragmatism of tests, it can be important to understand where they live within the perceived development workflow. Generally, it makes sense to attach fast, automated tests to precommit hooks in your version control system. Slow, automated tests can be postcommit and run on shared infrastructure. Very slow, exhaustive regression tests involving repeated, complex, time-consuming virtual infrastructure setup can be run when committing new tags/releases. It makes little sense for regular developers to focus on the latter two. However, the former is a great target for extra tests that's generally approachable by and visible to all. But code reviews can quickly become pissing contests used by people to impose their preferences on others: A passive aggressive way of asserting dominance on a team. You can see a lot of that in proponents of the 5 minute code review: No actual analysis of the code is done, but 5 minutes is plenty to use it as a tool for abuse. To avoid these issues it can be better to do these written/offline than in person. Maybe then discuss the results in person. So while the techniques described in the article can be very helpful, the wrong implementation of them will just help you standardize on a monoculture of bad engineering. So before implementing such things think about how good your engineering really is, and whether you really are better off making sure people bend to your standards, or you are better off learning from the experience of new people. Chances are you are not Google. Very well said.

jrockway 12y ago

You have to be careful about who you hire. If you do that well, then opinions, code reviews, one shared repository, and so on, work great. If you hire a bunch of assholes, then they won't work. I do about 20 code reviews a week; there is never bullying and they are only rarely rubber stamped. (My favorite code review of my own code this week was where I updated a counter to use atomic.AddInt64 instead of sync.Mutex. But where I was reading the counter, I just read the integer value directly instead of using atomic.LoadInt64. 30 seconds of the reviewers time, and we avoided a potentially difficult-to-track-down bug. No pissing match, no bullying. Just better programs.)

watwut 12y ago
Any company over 30 people will have at least one borderline asshole and one asshole is enough to poison the code review if managed wrong.
kabouseng 12y ago
20 code reviews a week? How many lines of code for each review?
- jrockway 12y ago
  I looked in more detail and if I average it out over 2.5 years and only include actual code I care about (not importing OSS packages, which I also review), it's more like one a day. I was too lazy to calculate a histogram, but let's go with ~100 lines a day.
  - kabouseng 12y ago
    Thanks for the reply.

EdwardDiego 12y ago

> Opinionated best practices stop being useful when the emphasis is on opinionated and not on best. If the cabal that has political influence is not the strongest there is, standardization just increases how much technical debt can be built in a short period of time. I agree so, so, so very much. We've had issues where one teams wants everyone to standardise on a given technology for ideological rather than practical reasons.