Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

How to run a miserable code review

badsoftwareadvice.substack.com

Post reply on HN
Print view View original

21–30 of 122 posts

Re: How to run a miserable code review

#21

Post by SilverBirch »

schwartzworld wrote: post #17

Code review is a flawed process, especially for junior engineers. You spend time and effort, maybe days of work, getting this to work, and then some know-it-all leaves a bunch of nit-picky comments or tells you you're doing it wrong. I tell most of the juniors I work with to work defensively against this using a few strategies: - smaller PRs. Break the work up any way you can. Make small tickets or submit your PRs wi…

Totally agree it's flawed, but very often the reason it's flawed is because something has gone wrong somewhere else. Yes, it's very common for someone to do a PR and for someone else to turn around and go "Oh god, why the hell did you do it like this?!" - Fundamentally challenging the entire purpose of the code. What's happening there is the code review is just highlighting the fact you didn't have a design review.

You also have to have a shared understanding of what's a reasonable request in a review, I've been in teams that relentlessly nitpick and teams that just "LGTM"s, eithers fine as long as you're agreed and consistent on what your team wants to do.

Re: How to run a miserable code review

#22

Post by ashtonbaker »

marktangotango wrote: post #4

I once worked at a place where about 5 of 15 developers sat on crucible all day fellating each other on code reviews. Anytime the rest of the developers would have code reviewed they'd be met with a long list of required changes that where "standards" the 5 had agreed on, and never communicated out side of their own comments in the tool. And the "standards" changed often, and the changes where never communicated eith…

What makes you call your current review culture “sane”? I’m not sure I have seen that yet in my career.

Re: How to run a miserable code review

#23

Post by bornfreddy »

asu_thomas wrote: post #14
adhesive_wombat wrote: post #5

Earlier quoted context omitted.

Bonus if the vague comments relate to some refactoring that you'd like to do, the details of which exist only in your head. Meanwhile, yes, you know the existing code is like X, and has been like that for years, but obviously I shouldn't have followed it and it's all wrong. For details on how I'd like it to be, please read your mind. Add an ellipsis to really drive home that only will any more explanations not be for…

> Bonus if the vague comments relate to some refactoring that you'd like to do I am extremely ashamed to say I have left a comment like this before. At the time, I was frustrated about never being able to discuss anything over a call. There is no excuse, though.

To be fair, it's not always a bad idea to present such ideas. However you need to make the comment resolvable, or better yet, create an issue for refactoring, add a small "fyi" comment and let them resolve it when they read it. This way they are aware of possible upcoming changes to this code but the MR/PR is not blocked.

Re: How to run a miserable code review

#24

Post by verall »

schwartzworld wrote: post #17

Code review is a flawed process, especially for junior engineers. You spend time and effort, maybe days of work, getting this to work, and then some know-it-all leaves a bunch of nit-picky comments or tells you you're doing it wrong. I tell most of the juniors I work with to work defensively against this using a few strategies: - smaller PRs. Break the work up any way you can. Make small tickets or submit your PRs wi…

> smaller PRs

I think the opposite works better - submit thousands of lines of changes, you will get like 10 comments, address all of them, and then you're gold. The reviewer has no time to slog through thousands of lines of changes unless the code you're touching is their baby.

Re: How to run a miserable code review

#25

Post by datavirtue »

SilverBirch wrote: post #21
schwartzworld wrote: post #17

Code review is a flawed process, especially for junior engineers. You spend time and effort, maybe days of work, getting this to work, and then some know-it-all leaves a bunch of nit-picky comments or tells you you're doing it wrong. I tell most of the juniors I work with to work defensively against this using a few strategies: - smaller PRs. Break the work up any way you can. Make small tickets or submit your PRs wi…

Totally agree it's flawed, but very often the reason it's flawed is because something has gone wrong somewhere else. Yes, it's very common for someone to do a PR and for someone else to turn around and go "Oh god, why the hell did you do it like this?!" - Fundamentally challenging the entire purpose of the code. What's happening there is the code review is just highlighting the fact you didn't have a design review. Y…

[flagged]

Re: How to run a miserable code review

#26

Post by digging »

datavirtue wrote: post #25
SilverBirch wrote: post #21

Earlier quoted context omitted.

Totally agree it's flawed, but very often the reason it's flawed is because something has gone wrong somewhere else. Yes, it's very common for someone to do a PR and for someone else to turn around and go "Oh god, why the hell did you do it like this?!" - Fundamentally challenging the entire purpose of the code. What's happening there is the code review is just highlighting the fact you didn't have a design review. Y…

[flagged]

Honestly, you sound like an asshole here. Do you realize that Adderall is a medication? Do you realize how fucked up it is to call someone mentally defective as if your brain meet some standard of perfect functioning? (it doesn't.)

Re: How to run a miserable code review

#27

Post by datavirtue »

beeskneecaps wrote: post #7

Triggered. Missing from this is the experience where the reviewer rejects for something trivial, then you fix the issue in 1 minute, resubmit, and finally have to wait 24 hours for them to check their code review requests again.

Or they open up the floodgates after that, listing numerous other unrelated "issues" with your code. In my mind you get one stab. After that STFU.

Re: How to run a miserable code review

#28

Post by kiitos »

schwartzworld wrote: post #17

Code review is a flawed process, especially for junior engineers. You spend time and effort, maybe days of work, getting this to work, and then some know-it-all leaves a bunch of nit-picky comments or tells you you're doing it wrong. I tell most of the juniors I work with to work defensively against this using a few strategies: - smaller PRs. Break the work up any way you can. Make small tickets or submit your PRs wi…

You've probably heard this before, but it's worth repeating: code review doesn't have to be a "flawed process" that is "nit-picky" -- if it is, there's a problem in your organization that needs to be -- and absolutely can be -- fixed!

Re: How to run a miserable code review

#29

Post by digging »

throw1234651234 wrote: post #3

There is a good article approaching it from the other side here: https://mtlynch.io/code-review-love/ - I am not the author, but it's one of the few things I have bookmarked.

I find this much more valuable than the main post, which is entertaining but leans too hard into absurdity to be a useful guide for what actual problematic behavior looks like.

Re: How to run a miserable code review

#30

Post by datavirtue »

digging wrote: post #26
datavirtue wrote: post #25

Earlier quoted context omitted.

[flagged]

Honestly, you sound like an asshole here. Do you realize that Adderall is a medication? Do you realize how fucked up it is to call someone mentally defective as if your brain meet some standard of perfect functioning? (it doesn't.)

No one cares after they tumble over the edge and make everyone miserable. It ain't helping anymore.
Post reply on HN