Ask HN: What do you hate most about code reviews? | Hacker News Reader

Ask HN: What do you hate most about code reviews?

2 pointszevir10 comments

Code reviews have become a pretty standard part of how dev teams operate. But I've talked to a bunch of developers about this recently, and it seems like everyone wishes the process was different/better. For some it's an emotional thing - no one likes to get critiqued. For others it's a procedural thing - their company doesn't have clearly defined procedures in place for doing code reviews. So I'm turning to the HN community for some expert feedback. What do you hate most about code reviews? What would do do differently do make it better/more effective?

Thanks!!

View on HN

Maybe an obvious one, but at my last org I often wished for more emphasis on keeping patch sets SMALL. I might have liked some kind of soft ban against putting huge patch sets up for review. I'm talking like 1000+ line stuff. In my experience, no one wants to review those; it's incredibly tedious.

I say "soft" ban because there are exceptions. I don't mind huge patch sets if the changes are proportionately trivial: One example is mechanical search-and-replace jobs on large codebases. But the 3000-line new module that just gets dumped on reviewers all at once is in bad taste. It's the responsibility of the author of the code to ensure that it is broken up in a way that makes reviewers' job tolerable. My opinion.

This would happen frequently at a previous job, and it bothered me. Thousands of lines of changes in a repo where you have very little context, not just about the change, but about the project as a whole. While you're trying to learn about the project, submitters are hounding you with Slacks to get the review in so they don't miss some arbitrary sprint deadline.

If code reviews are to be done seriously, time should be allocated / scheduled properly (for the reviewer.)

Thanks! Do you think it would help if you had a live preview environment that came with every PR, so you could better understand the context of the changes you are reviewing?

That would help, definitely! Unfortunately, I think the overhead would be too much for some organizations.

Thanks! Great point

Review is often a judgemental concept. It really should be about collaboration and compromise. So I would expand on the tools to compromise, and not just a change list

I agree 100%. The question is how to facilitate this collaboration/compromise between developers. We've actually built a developer collaboration platform called Livecycle (https://livecycle.io/) that lets developers collaborate/comment on a live PR preview environment. Do you think this could help with the pain point you are describing?

A website of the changes is useless for any amount of complexity as you can't step through the code properly.

Thanks! Do you think it would help if you had a live preview environment that came with every PR, so you could better understand the context of the changes you are reviewing?

I'm old school which means tags and cscope. I need to be able to walk the function paths of complex code in a terminal. I simply don't see how other folks can review code by staring at pages of code without similar context.

Reply on news.ycombinator.com