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.)
I was too lazy to calculate a histogram, but let's go with ~100 lines a day.
In short, code reviews don't have to be bad, from personal experience.
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.
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.
If/when you start working professionally, you'll learn why my initial comment got upvoted.
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.
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.
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.