Hacker News new | ask | show | jobs
by pvnick 4614 days ago
This post hit home for me. I've personally implemented code review strategies which directly and immediately led to much improved code quality and generally better product. But management doesn't see code quality. They see deadlines. The insignificant time it takes for code review is the first thing that gets nixed by non-technical management even when the time required for bug fixes, last-minute changes due to their own indecisiveness, and slow development due to giving employees second-rate hardware far eclipse the marginal time it takes to make sure we're deploying halfway decent code to production. And then they bitch at the engineers that they're underpaying/overworking when shit stops working. But let's not hire more developers and improve salaries for the people we have. No, that's not what we need. What we need is more charismatic biz-dev bros with poly-sci degrees. Surely that will fix things! (/rant)
3 comments

It seems like code reviews are partially for inspecting the product, and partially for teaching and inculcating cultural norms.

Are the managers who don't like this just non-technical? Or are they just not being presented the value in a clear enough way?

> partially for inspecting the product, and partially for teaching and inculcating cultural norms

Yes!

I would go further and say that - without discounting their value for catching bugs - the _largest_ benefits of doing code reviews are cultural rather than technical.

You pair with people you like and do code reviews with people you don't like (misquoted from someone way smarter than me).

With large teams, especially if distributed or partially outsourced, code reviews can ensure code quality. But also be a total bottle neck if over-bureaucratic and some reviews are of low quality due to lack of context. Often combined with ivory tower architects as well.

In smaller, agile and especially collocated teams code reviews will flag issue unnecessarily late in the process. Just pair from the start instead to ensure no short cuts or dodgy code slips through, and automatically spread the knowledge. If you do not trust two of your developers combined then you do have a serious problem.

You can though in addition have small and short swarming/tripling/quadrupling sessions in front of 1 computer to look at especially important issues.

If you do neither code reviews nor pairing then you are in trouble.

Can you expand on what you mean by this? What cultural benefits are you talking about?
One of the huge benefits I've found from code reviews is that my reviewer will say, "Actually, we've faced that problem before and there's a solid and proved solution in our utils that handles it already, along with a couple other cases. How about you change it to use that instead?"
Yes - building on this, I see 4 quick benefits:

1 - Suggestions on where problems have been solved before. (Your point)

2 - Having people say, "Here's the style we use to make this easier to support in the future"

3 - Mentoring on tougher problems, and turning quick hacks into elegant solutions.

4 - QA. (This is the stated benefit, but falls below the other 3)

The key is not to turn code reviews into a bottleneck. If you just view the purpose as any 1 of the 4, you're likely to under-prioritize or over-formalize it.

Additionally, just knowing that code reviews are going to happen often results in developers putting in more effort to submit quality code.
The word "non-technical" seems generous. How about newb or non-functioning or subhuman? Systems are great, but as you point out it's really the people that matter.
> The insignificant time it takes for code review is the first thing that gets nixed by non-technical management

You have to fight back against that shit. Testing, reviews, these things are part of the job. You're the expert, you tell them how long it takes to do things. Don't let them just hand you a deadline without saying something. Speak up!