Ask HN: What do you hate most about code reviews?
3 comments
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.
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.)
If code reviews are to be done seriously, time should be allocated / scheduled properly (for the reviewer.)
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.
Thanks!!