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

11–20 of 122 posts

Re: How to run a miserable code review

#11

Post by fuzzy2 »

notjustanymike wrote: post #2

So many missed opportunities for a small business. A code review should never be done in person; instead all communication should happen asynchronously through passive aggressive messages left in GitHub. If you're on bi-weekly sprints, the review should be left to age for a minimum of 5 business days before any feedback is provided. Never write resolvable comments, but rather leave ambiguous musings of how a block of…

What, those are rookie numbers! I opened a pull request 13 business days ago. I even rebased it multiple times when conflicting code was otherwise merged. There’s been no feedback whatsoever. Everyone even remotely involved is now on vacation.

(It’s not like I care either, the project sucks anyway.)

Re: How to run a miserable code review

#12

Post by pak9rabid »

notjustanymike wrote: post #2

So many missed opportunities for a small business. A code review should never be done in person; instead all communication should happen asynchronously through passive aggressive messages left in GitHub. If you're on bi-weekly sprints, the review should be left to age for a minimum of 5 business days before any feedback is provided. Never write resolvable comments, but rather leave ambiguous musings of how a block of…

And in the case of merge conflicts, steamroll your changes over everyone else's, as you know what's best and everybody else is wrong.

Re: How to run a miserable code review

#14

Post by asu_thomas »

adhesive_wombat wrote: post #5
notjustanymike wrote: post #2

So many missed opportunities for a small business. A code review should never be done in person; instead all communication should happen asynchronously through passive aggressive messages left in GitHub. If you're on bi-weekly sprints, the review should be left to age for a minimum of 5 business days before any feedback is provided. Never write resolvable comments, but rather leave ambiguous musings of how a block of…

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.

Re: How to run a miserable code review

#15

Post by agentultra »

notjustanymike wrote: post #2

So many missed opportunities for a small business. A code review should never be done in person; instead all communication should happen asynchronously through passive aggressive messages left in GitHub. If you're on bi-weekly sprints, the review should be left to age for a minimum of 5 business days before any feedback is provided. Never write resolvable comments, but rather leave ambiguous musings of how a block of…

Minimum 5 days hurt. Too real. If there’s one way to destroy productivity and morale it’s to leave a vague comment on a review and not come back to it for a few days.

Re: How to run a miserable code review

#16

Post by agentultra »

Also, if you have a bad manager who likes to gaslight you with questions that make you doubt yourself, a code review is a great way to pass on the good feelings to your co-workers. Make sure to ask plenty of leading questions that interrogate the competence of the author. The more passive-aggressive the better. And word it so that you can deny anyone that calls you out for negative behavior.

I don’t understand this line here, could you tell me why you decided to go this route?

Why are you using this function? Do you not know about the ?

Make sure to leave the author wondering what they could have done differently with their lives in order to avoid having to interact with you. But word it so harmlessly that they can’t tell if you’re genuinely interested in helping the team ship code or whether you’d push them under a bus if it helped you cross the street.

Re: How to run a miserable code review

#17

Post by schwartzworld »

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 with Part 1, part 2, etc. You can't release half-baked work, but you can usually find opportunities to split things up. Nobody should be submitting 20 file PRs and not expecting a lot of comments.

- Validate your strategy before building it. If there's a senior engineer in your team who nitpicks your code, get their buy-in before writing it.

- often bad code is the result of not really knowing how to approach a problem. Don't just write the code and slap together a PR. You might need to figure out a working solution, then go back and tweak it and refactor it before submitting it.

- review your own PRs and use a critical eye. You'll catch the low hanging fruit (like forgetting console.log calls). Every time you have the urge to leave a comment justifying a choice, question whether it's the right choice at all.

Re: How to run a miserable code review

#18

Post by cjfd »

agentultra wrote: post #15
notjustanymike wrote: post #2

So many missed opportunities for a small business. A code review should never be done in person; instead all communication should happen asynchronously through passive aggressive messages left in GitHub. If you're on bi-weekly sprints, the review should be left to age for a minimum of 5 business days before any feedback is provided. Never write resolvable comments, but rather leave ambiguous musings of how a block of…

Minimum 5 days hurt. Too real. If there’s one way to destroy productivity and morale it’s to leave a vague comment on a review and not come back to it for a few days.

5 days is very quick. I have an open pull request that has been open for 6 months or thereabouts.... :-( No, I am not suggesting this as a good practice.... Actually, I would say that a pull request should be reviewed in 24 hours. Pull requests made on Friday may wait until Monday.

Re: How to run a miserable code review

#19

Post by PartiallyTyped »

How to make your colleagues miserable 101.

    1. open a cr,
    2. have your inexperienced friend approve it,
    3. have the lead eng add 15 comments they caught,
    4. merge the code anyway despite it breaking the core feature that’s being merged in parallel because you can’t just follow the design of the person who thought everything through and decided you knew better,
    5. waste 8 more hours of your lead while they pair program with you, explain the consequences of your changes as you start to grasp that there’s a lot you don’t know, reverting many of the changes, implementing a cleaner approach, understanding why the lead commented what they did,
    6. and hope they won’t get mad when they go back to fix merge conflicts on things that shouldn’t have been touched as they try to build the core..

Re: How to run a miserable code review

#20

Post by danparsonson »

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…

Alternative view - if you approach a code review (or indeed any similar process) as a competition to be won or some kind of personal validation process, then you're likely to be disappointed and are potentially wasting an opportunity to learn something. Especially as a junior!
Post reply on HN