Live data from Hacker News

I ruin developers’ lives with my code reviews and I'm sorry

habr.com

161–170 of 199 posts

Re: I ruin developers’ lives with my code reviews and I'm sorry

#161

Nothing makes working at a place more unbearable than having to deal with someone that gives an excessive amount of criticism on working code, at least that's how I feel. I've been in that situation before and it made me never want to submit pull requests. It made me even madder when other developers, whose code was no better than mine but had been at the company longer, received basically no critiques. Some might sa…

Strange, I’m on a team rn with people who just approve everything. I’m craving some constructive feedback.

It might mean you're doing it basically correct, or it might mean you're touching parts that people don't care about much. The second is probably more common.

(It might also mean your team doesn't care about anything any more, in which case look for a better opportunity, unless you want to just cruise along for a while for whatever reason, in which case turn into a bare minimum performer like the rest of your team).

I'm a senior engineer in a known company and my target is always to leave no comments on a patch. I only give question-comments if I absolutely must know something before merging, or please-fix comments if something is definitely wrong and I can prove it. Also, if it's a part of the system that poses little risk (for example, if it breaks I can assign the bug back to the author and patiently wait for a fix with no serious damage) my tolerance for bugs greatly increases. The last part I'm not too proud of but frankly I don't have time to review everything. A "fix" for the last part would be to find a lower rank reviewer-buddy who has more time to share knowledge.

Re: I ruin developers’ lives with my code reviews and I'm sorry

#162
If you're giving soul-crushing feedback and not enjoying it like the OP, one problem could be that perhaps you're not providing the developer with enough detail on the task at hand. Communication isn't easy.

I have been mentoring junior developers for about 10 years, and some more senior ones for the last few years. My ego is more attuned to getting things done than tearing up people's work. The following works really well for me:

1. Never insult people's work, it's counter-productive. (Unless see number 9) 2. Nitpick on convention/style, not implementation, unless the implementation is infeasible. Convention and style will quickly become rote. This forces new developers to pay more attention. 3. If it works as intended and has meaningful tests, MERGE IT. Congratulate them :rocket:! 4. Point out logical pitfalls/caveats by asking "what would happen if". Asking questions starting with "why" triggers people and puts them on the defensive. That's your EQ lesson for today. 5. Through code-review, you can identify a lot of little problems and suggest articles to read that would help the person grow. Examples: misunderstanding async/await vs. thenable in JS. Parallel programming in R/Py. These are distinct, language specific (R has sessions/multicore, Py has the GIL) pieces of knowledge that people can learn easily with a tiny bit of guidance. 5. If you're a better programmer - write the first 1000 lines so that they can fill in the next 10,000 (credit: don't remember, someone tell me?) 6. Even junior developers can get a surprisingly challenging task done if you give them hints (point form) about how you might approach it. 7. Do code reviews in-person (1-1) or if you are tactful with your feedback and have encouraged an environment where making mistakes is acceptable - in a group, that way everyone learns! This is the management version of the DRY principle. 8. One of the biggest problems I see with new developers is "how do I break this problem into smaller pieces". Discuss potential approaches BEFORE the PR. Not after. 9. Fire people that are consistently lazy. When you manage people for a long time it easy to see the difference between someone that needs training and someone that is lazy. You can help a someone that wants to learn, but you can't motivate someone to be interested in something that they aren't already. 10. Cultural differences can make people less likely to speak up when they are stuck - check in with your team!

Re: I ruin developers’ lives with my code reviews and I'm sorry

#163

> If a guy brings me his code, and it has mistakes, it brings insane pleasure from how smart I feel [...] And if you tell me that you haven’t had this feeling ever, then you’re lying. Tell me about higher goals, training rookies and all that — I know you’re simply too full of themselves. And if you try to tell me that you learned to defeat that feeling (however it manifests in you), then I must be a pink unicorn. I r…

> I'm constantly looking for ways to do less tedious work, through automation and such, and code review for bad code is the worst sort of tedious work.

What kind of automation or tools are you using for code review?

Re: I ruin developers’ lives with my code reviews and I'm sorry

#164
post #84

> If a guy brings me his code, and it has mistakes, it brings insane pleasure from how smart I feel [...] And if you tell me that you haven’t had this feeling ever, then you’re lying. Tell me about higher goals, training rookies and all that — I know you’re simply too full of themselves. And if you try to tell me that you learned to defeat that feeling (however it manifests in you), then I must be a pink unicorn. I r…

I have worked with people like the author. They suck. They destroy entire teams and often times the feedback is 99% pedantic personal preferences because why would they do something useful like setup the linter and propose the silly things they always nit pick. These people don’t last long and I have gotten pretty good at screening out this personality in interviews after having it cause huge issues at two different…

I think I've had this author review my homework project for several job interviews.

Re: I ruin developers’ lives with my code reviews and I'm sorry

#165

Earlier quoted context omitted.

For two decades I wrote professionally and there were no code reviews at all. I had no problem with that.

I did it for three decades (3.5?) and the only code reviews were with friends of mine who worked at multiple companies together. The whole point of the review was to find outright errors, not to optimize. Scarcely any ego involvement. It worked out fine I think and, mind you, it used to be a lot harder to push out a fix. A lot harder.

Yeah, maybe I should add, we would hang out in the hall and whiteboard our ideas, workflows before we went back into our respective cubicles/offices and started coding.

The code review was preloaded.

Re: I ruin developers’ lives with my code reviews and I'm sorry

#166
> And finally, I became the exact thing I hated: a toxic asshat throwing his skills around like fists. I don’t do code review for the business, I just like showing the rookies their place. My skills have finally started to pay off.

Or he's just lazy.

It's way harder to build up the people you work with than tear them down. The key ingredients tp the former are patience and empathy. And those qualities are lacking in many managers.

Also, aggressive, opinionated people are often perceived as more intelligent than their more reserved peers. You'd think that the snowmen can't get snowed, but the type of person the author describes seems to be just as vulnerable to the misdirection as others.

Re: I ruin developers’ lives with my code reviews and I'm sorry

#167
post #115

Earlier quoted context omitted.

I'm curious to know if you or anyone else reading this has run into the opposite problem. Or maybe I'm just "that guy" although I am reasonably certain that I am not. As a rule, so far throughout my 5 year career as a full stack web developer, my coworkers have neither understood or cared about what clean code entails. I don't berate, I don't criticize, I've taken to mentioning it once and then doing my utmost to nev…

Your team is fairly typical, for at least a large subsection of the industry. > But the devs, man. They just don't care. Well, maybe. Or they just don't know how to write better code. Either way, the bigger problem is the team management that allows this. > I do my best to avoid rewriting wherever possible Why not? If the other devs truly don't care, you could take it upon yourself to clean up the code base little by…

> Why not? If the other devs truly don't care, you could take it upon yourself to clean up the code base little by little when you can.

I used to do this simply because it makes the code difficult to read for me to review if for nothing else.

Linters and style check tools only go so far, and bottom line is you have to ship software not write complex and expensive to maintain tooling, so automation beyond a point is not a solution.

The devs ended up relying on me as the their clean up guy for all the code all the time. I also becoming gatekeeper/bottleneck of sorts.

It is used make blame and dissection more difficult as more times than not I had cleaned up the code and the last one or two commits are mine.

Also the time spent in reviews reduce and you tend to miss important things as you have focused on what should be basic hygiene so much.

One of the reasons I contribute a lot less to production code is this. It is a losing battle if most devs don't care about it as much as you .

Even now the occasional pull request I would make would be peppered with such corrections , it is just automatic reflex for me, but it doesn't make that much difference.

Don't do this, people either have to value the cleanliness enough to do it as well, or you have to tolerate what is the common minimum or find another team/organization.

Re: I ruin developers’ lives with my code reviews and I'm sorry

#168
post #115
post #84

Earlier quoted context omitted.

I have worked with people like the author. They suck. They destroy entire teams and often times the feedback is 99% pedantic personal preferences because why would they do something useful like setup the linter and propose the silly things they always nit pick. These people don’t last long and I have gotten pretty good at screening out this personality in interviews after having it cause huge issues at two different…

I'm curious to know if you or anyone else reading this has run into the opposite problem. Or maybe I'm just "that guy" although I am reasonably certain that I am not. As a rule, so far throughout my 5 year career as a full stack web developer, my coworkers have neither understood or cared about what clean code entails. I don't berate, I don't criticize, I've taken to mentioning it once and then doing my utmost to nev…

It’s certainly possible they don’t care, or they don’t know how to write good code.

Another possible reason could be that they know it’s temporary. A lot of startup code gets rewritten at least once in the first 5 years, there is some art to knowing what code might stick around and be worth the effort to do well, and what you should just bang out fast and move on.

It sounds like it’s possible you haven’t been able to have a conversation with your colleagues about their choices, which might be worth having candidly if you can find the opportunity to do so. If it is ignorance or negligence better to know and decide how you want to proceed if that makes sense.

Re: I ruin developers’ lives with my code reviews and I'm sorry

#169
tried to use bitrix like 6 years ago or so and it was hell, they said absent documentation should be looked for on their forum, and yes it was there, but so much hate even from topic starters, got out 1 month later and it was never too early. im russian myself, i will never work with russian dev products ever.

Re: I ruin developers’ lives with my code reviews and I'm sorry

#170

Earlier quoted context omitted.

That is exactly why many harshly critique code. But I've never found their versions to be any less full of tech-debt. Usually it comes down to personal style. "Oh Brenda hates if/else, better convert it to ternary before I submit it. But Oh No! Bob is reviewing it instead! He hates ternary and always wants if/else! I'm doomed..."

If code reviews are centered around style the entire org is fucked.

[deleted]
Post reply on HN