Comment by spankalee
Comment by spankalee 3 days ago
I stopped reading after that opening paragraph. I don't know of anyone I take seriously who thinks that code reviews are bad practice or pure red tape.
Comment by spankalee 3 days ago
I stopped reading after that opening paragraph. I don't know of anyone I take seriously who thinks that code reviews are bad practice or pure red tape.
Ask 5 people about the purpose of mandatory PR reviews and you’ll get 6 answers.
However catching bugs is always going to be at or near the top of list, so clearly it’s at least partially about catching bugs.
I’d argue that catching bugs along with ticking a compliance checkbox (which is only there because something thinks they catch bugs and malicious code) are the 2 primarily reasons that the business part of the company cares about or requires code reviews in the first place.
I know an idi*t who claimed, in a code review, that there was a memory leak just by looking at the code (turned out there wasn't). Clearly it was a bullying attempt to stop someone else's progress. Unfortunately it was successful because of people like the ones downvoting you.
Mandatory code review definitely creates red tape. Every place I've been with mandatory code review, I always see people "looking for a stamp".
At my current job, code review requirements are set on a per-folder basis, with many folders not requiring a review. People ask for a review because they want a review (or, sometimes, they dont. For example, I don't ask someone to review every quick one-liner of the systems I am an expert in).
Sure there's some subset of commits where all that makes sense is a stamp. On good teams it at least still ensures that two people agree that a stamp is appropriate. Knowing that you need to be able to convince someone it's good before it goes in is a good forcing function reducing sloppiness and laziness.
> On good teams it at least still ensures that two people agree that a stamp is appropriate.
That is a good point!
Then you are very lucky. I definitely have met those sorts. I’m even aware of teams that collectively push to the main brain under the promise that they’ll probably look at each other’s code later, maybe.
I saw no proof of the later review for all pushes.
Same.
> Code reviews have a bad rap: they are antagonistic in nature and, sometimes, pure red tape.
I wonder if folks know that this is a job? What are you gonna do, not do it? Cry at night because you forgot for the hundreth time to add token strings to translation files and not be hard-coded? Come on.
A few days ago there was an article on HN on how engineers abuse code reviews. It's just a tool, the outcome is different based on who's reviewing. If you think code review is intrinsically good then I'm glad I'm not working with you either
You would be surprised! I have encountered the attitude that code reviews are a waste of time. It's not common, and I have never seen this attitude "win" across a team/company but it definitely exists. Some engineers are just overconfident, they believe they could fix everything if everyone would just let them code.
I’ve never worked somewhere where mandatory PR reviews didn’t turn into mostly red tape.
The pressure to get work done faster in the long term always wins out over other concerns and you end up with largely surface level speed reviews that don’t catch much of anything. At best they tend to enforce taste.
In 20 years across many companies and thousands of PRs, I’ve never had a reviewer catch a single major bug (a bug that would have required declaring an incident) that would have otherwise gone out. I've pushed a few major bugs to production, but they always made it through review.
I’ve been doing this since well before everyone was using GitHub and requiring PR reviews for every merge. I haven’t noticed a significant uptick in software quality since the switch.
The cost is high enough that I’d like to see some hard evidence justifying it. It just seems to be something people who have never done any different take on faith.