Live data from Hacker News

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

habr.com

61–70 of 199 posts

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

#62

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…

IMO the absolute worst is excessive criticism while missing a serious business logic or functionality bug.

In my opinion the first priority of code review should be to prevent mistakes in the code at hand

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

#63
I had an experience in my first year of grad school that has informed my code reviews and my general attitude toward reviewing others' work ever since.

I was taking a graduate-level introduction to mathematical logic. In the class we learned about a bunch of different propositional axiomatic proof systems, and then we proved properties like soundness and completeness for them. The class was organized around class participation: everyone was required to do at least one proof at the board in front of the class and every session consisted almost entirely of students doing proofs at the board.

I'd been out of school, working, for eight years before grad school and I found this all very intense and unfamiliar. I did my required work at the board, choosing an easy problem, but otherwise mostly sat in the back and watched. There was, however, a cohort of very intense aspiring logicians who sat in the front row and took deep interest in every proof.

One day, a student went up with a particularly challenging problem. From the start it was clear that he was taking an unorthodox approach to the proof. The front row went nuts, interjecting and kibitzing on practically every line, telling the guy different things that he should do instead of whatever it was that he was doing. The guy was a good sport and tried to justify everything and answer all the criticism, but it seemed like he might not get through the proof before the class ended.

The prof who had been watching quietly from the side of the class, let it go on for a while, then stepped out and quieted everyone.

"I need you to remember," he said to the front row, "that we are the audience. It is not our job to prove this theorem; it is this gentleman's job. Our job as the audience is to listen to him, and at the end, decide if we are convinced. He may do things differently that you would have done, but that doesn't matter, as long as in the end we are convinced."

That was twenty-three years ago, but it stuck with me. In programming, there are often many different ways that a problem could be solved, but there is almost never One True Way™, even though there are often many voices that would like to convince you that their way is the right one. My job as a reviewer of code, or really anything, is not to try to get the other person to do what I would have done, but to decide if I'm convinced that the way that they've chosen is good enough, and, if not, to ask for changes that would convince me.

Sure, sometimes a PR is so bad that it needs to be completely rewritten, but in my experience that's rare, and in that case the feedback should probably be given in person and in private or in a friendly, sympathetic group.

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

#64

> 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 was mad that, while I spent my nights learning F#, my daughter started calling everyone around “fathers”. And this guy, instead of getting better at his job, went home to his children. And I wanted to punish him.

This reads like American Psycho. I can't believe this guy is for real. You have nothing but your job, so - at your job - you try to make everyone you work with miserable? What? Why?

Have you considered trying to make your life enjoyable ever?

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

#65
This except there is no right way and there are 'n' equally good ways of solving any real world problem. It's only worth pointing out flaws if something is genuinely bad and I mean bad. Otherwise you are using a gut opinion to trash someone else's emotional health.

Yay! you found an edge case that breaks a piece of code that will never get hit while the other million and 1 edge cases that haven't been found are still out there. Yet you drag it out from the dark and show it off for all to see like some damn trophy. Yay! look I am great I found one more thing wrong than the other guy. No matter that we are all swimming in a sea of wrong and every one is completely clueless. Only slightly less clueless than those who don't even realise they don't have a clue.

It follows from a simple argument - every programmer and every company has a different view of 'good'. I've seen it, I've worked in many. Just as every religion cannot be right, every weird view on correct programming style cannot be correct.

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

#66
post #2

Congrats to the author for realizing that confidence doesn't equate skill. This isn't being a pink unicorn, it's simply humility. The real danger of confusing the two is that real skill dulls if you let overconfidence take over, since the very thought of being "better than others" prevents you from accepting that maybe there are things that you don't know or haven't considered. For example, is "destroying" a coworker…

If you have to "destroy" every PR from a developer, you're wasting more time (money) than it would take to replace them. Why should be bad PRs ever be allowed for any reason?

A bad PR can mean soo many different things.

Ranging from "I wouldn't have implemented it that way" to "this opens up a huge security hole".

Security issues shouldn't be allowed in. But a lot of "maintainability" issues have negligible if any gains, and real costs like

it takes time to fix

could introduce new bugs

creates bad feelings between coworkers

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

#67
I really haven't coded for long compared to some.

I always look at code reviews or just reading / talking about each other's code as a way for us to just get on the same page about how we do things. Make everyone's life easier. "Hey man when we're looping through that thing here is how we usually do it." "Ok cool, I'll do that." Or maybe we have a discussion on why I did it differently this time and how we might address that.

Much of the time I don't "care" or feel strongly what the change or answer is. If we're all doing the same thing / spot a bug sooner, it's just easier for everyone.

But it seems code reviews to some are this sort of battle / proving ground / have a lot of elements of status. I don't get that...

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

#68
There are a few types of comments on code review that I see:

1. Critical mistakes

2. Style guide mistakes

3. Non-optimal design (in your opinion)

#1 Always receives a comment, obviously.

#2 Always receives a comment, but we have good automation here so it's not too much of a worry.

#3 Is where it gets interesting. A lot of the time, commenting about this category is prematurely optimizing and it's a waste of time for both the reviewer and the owner of the change. At most I try to make one overarching comment about the design.

You really have to ask yourself "Does it work? Will it cause issues down the line?" If the answers are Yes and No respectively, you should likely let it go.

The feeling of ownership over the design, especially for a junior engineer, is incredibly important for moral and the ability to get better over time. IMO it's also important to let them make small mistakes now and again so they can learn from it.

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

#70

> 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 was mad that, while I spent my nights learning F#, my daughter started calling everyone around “fathers”. And this guy, instead of getting better at his job, went home to his children. And I wanted to punish him. This reads like American Psycho. I can't believe this guy is for real. You have nothing but your job, so - at your job - you try to make everyone you work with miserable? What? Why? Have you considered t…

*Russian American Psycho
Post reply on HN