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

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

habr.com

Post reply on HN
Print view View original

71–80 of 164 posts

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

#71

Post by lainga »

iheartpotatoes wrote: post #8

This is incredibly honest. Maybe it's just me, but it 100% resonates with me. I find myself doing the exact same thing: as soon as I feel myself mounting up on my high-horse I have to talk myself down from being an asshat. I've gotten better at it because I'm now responsible for people's careers, and my pettiness is no reason to make someone's life suck because they are still learning -OR- because they think differen…

Oh lord, the stickers. I felt an interview with a local startup turn south when I pulled my Thinkpad out and was asked "where are the stickers?".

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

#72

Post by iheartpotatoes »

Fellshard wrote: post #26
iheartpotatoes wrote: post #8

This is incredibly honest. Maybe it's just me, but it 100% resonates with me. I find myself doing the exact same thing: as soon as I feel myself mounting up on my high-horse I have to talk myself down from being an asshat. I've gotten better at it because I'm now responsible for people's careers, and my pettiness is no reason to make someone's life suck because they are still learning -OR- because they think differen…

I don't think diversity of skin color and gender will solve this problem in its entirety too. People of every race and every gender have the capacity to be this kind of self-centered egotist. People of every race and every gender have the capacity to be excellent developers who are focused outwards, on developing their teammates and building things well. Diversity is part of the picture, yes, but it's no instant win,…

Why did you feel the need to state the obvious? This is the kind of toxic nonsense that comes up every time someone mentions diversity. Its the status-quo "notallmen" argument again and again and again.

Please, assume just for a minute that we aren't all idiots. Of -course- everyone has the capacity to be an idiot, but your unnecessary and highly defensive response is very telling.

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

#73

Post by remote_phone »

I am an notoriously tough code reviewer. Not because I’m mean, but I’m thorough and consistent. I’m never passive aggressive and simply factual. But newer team members can find dozens of comments until they understand what my expectations are.

I can generally differentiate between personal opinion and bad form. I rarely comment on things like variable names, or function names unless they are really confusing or misleading. I have my own style, but the existing style of the code should be what we code based on, not personal opinions.

I think the OP is shirking their responsibilities for what they’re being paid for. If they are doing a code review it’s for the benefit of the team, so that the team member understands what their expectations are. You don’t have to be mean to be a good code reviewer.

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

#74

Post by joaomacp »

aristophenes wrote: post #13

"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." I can tell you I haven't had this feeling ever, but not for reasons I'm proud of. I hate doing code reviews. I have hated doing every one. I have disliked having to type every comment I have made on a code review. Each time I hope that ev…

> "If a guy brings me his code, and it has mistakes, it brings insane pleasure from how smart I feel"

This quote made me laugh from the exaggerated "insane pleasure" in other's mistakes. But I feel this way sometimes. I'm completely sure it comes from my own insecurities, so that when I see another guy missed something I wouldn't, I get a feeling of relief - actually I'm not the worst programmer in the world.

So for me this feeling comes from impostor syndrome - I thought I was bad, I saw another guy who was worse, I felt good. Insecurity is really a self-fulfilling prophecy. I'm trying to work on it by forcing myself to ask questions I think are dumb, and not fearing judgment from others. This in turn will also influence my peers to feel less afraid of their doubts, turning the spiral around.

A study from Google [1] agrees with this: they found that the best predictor of team quality was the willingness to ask questions and express opinions without fear. Quoting Google:

"In a team with high psychological safety, teammates feel safe to take risks around their team members. They feel confident that no one on the team will embarrass or punish anyone else for admitting a mistake, asking a question, or offering a new idea."

[1] https://www.inc.com/justin-bariso/google-spent-years-studyin...

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

#75

Post by Hydraulix989 »

sauceop wrote: post #56
fredsted wrote: post #10

I've seen a few cases where otherwise talented developers would kind of miss the point of code reviews and focus on code style much more than the code itself, nitpicky stuff like sorting of imports, etc., leaving hundreds of comments while at the same time overlooking quite serious bugs. Presumably, codestyle comes easy for them due to their neurotype, but they have a hard time reining themselves in and just end up w…

Consistency of code style does matter, up to a point, to people reading and understanding the code. As you suggest, setting up ground rules and tooling helps Code authors who don't see the value in consistency of code are potentially a problem - if authors are submitting code reviews with hundreds of actual style issues, that's either a failure of process or the author to write readable code (not sure if that's what…

It could also be a testament of the reviewer's inability to read code written differently.

Style is not consistent across authors, codebases, projects, even companies. I had to learn to read code in many different styles.

Enforcing consistency can be done with formatters/linters. It doesn't need to come up in review.

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

#76

Post by sauceop »

rb808 wrote: post #30

BTW does anyone have a good guide, book, tutorial on doing code reviews? Everywhere I've been its kinda adhoc with no rules or guidelines. Often code reviews come in after someone has put a few weeks of work into something and its too late to change how it was done. Or a review is passed in by someone in a hurry without actually critiquing it.

We had a senior engineer (who sadly moved on to a different company) periodically give a talk to new hires about good code review practices to set expectations and ground rules. Some of the key points:

* Authors are expected optimize for readability of code, even if it takes longer to get the code in.

* Reviewers should generally only start reviewing code if they are ready and willing to continue the review to completion (i.e. no drive-by reviews). Once they start reviewing they should try to minimize response time.

* Focus the reviews on testing, correctness, readability

* Consistent code style is important (see optimizing for readability)

* Break code reviews into the smallest reviewable units. Often these are somewhat large because parts of the change don't make sense in isolation

* The code author is responsible for writing code in a way that it convinces the reviewer that it is correct (see optimizing for readability)

* You should approach it collaboratively - you're working together to get the work done and make the codebase as maintainable as possible

* You should aim to leave each bit of the codebase that you touch in at least a good a state as it was previously

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

#77

Post by dfilppi »

Great article. I suspect most that have gained a level of proficiency have felt that pull and acted on it to some degree. The best thing for you now would be to be hopelessly crushed by someone far more capable. Or get fired for being an asshat at a vulnerable time in your life. Then you may achieve the humility and confidence to become a true leader, which requires you to allow others to feel strong.

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

#78

Post by lainga »

iheartpotatoes wrote: post #72
Fellshard wrote: post #26

Earlier quoted context omitted.

I don't think diversity of skin color and gender will solve this problem in its entirety too. People of every race and every gender have the capacity to be this kind of self-centered egotist. People of every race and every gender have the capacity to be excellent developers who are focused outwards, on developing their teammates and building things well. Diversity is part of the picture, yes, but it's no instant win,…

Why did you feel the need to state the obvious? This is the kind of toxic nonsense that comes up every time someone mentions diversity. Its the status-quo "notallmen" argument again and again and again. Please, assume just for a minute that we aren't all idiots. Of -course- everyone has the capacity to be an idiot, but your unnecessary and highly defensive response is very telling.

As much as I am tempted to, I don't think it's right to equate diversity of appearance and diversity of experience, either. One is just a proxy for the other.

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

#79

Post by dfilppi »

skybrian wrote: post #54

As a professional, I got tired of being mean, so I switched to a different review style where I ask questions instead. I would still ask a lot of questions. Then I started to worry that I was coming across as passive-aggressive, that these suggestions would be taken as veiled demands to fix things. As a countermeasure, sometimes I'd explicitly say things like, "you can submit this as-is, but here are a bunch of thing…

That's the socratic method. Far more constructive.

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

#80

Post by sauceop »

Hydraulix989 wrote: post #75
sauceop wrote: post #56

Earlier quoted context omitted.

Consistency of code style does matter, up to a point, to people reading and understanding the code. As you suggest, setting up ground rules and tooling helps Code authors who don't see the value in consistency of code are potentially a problem - if authors are submitting code reviews with hundreds of actual style issues, that's either a failure of process or the author to write readable code (not sure if that's what…

It could also be a testament of the reviewer's inability to read code written differently. Style is not consistent across authors, codebases, projects, even companies. I had to learn to read code in many different styles. Enforcing consistency can be done with formatters/linters. It doesn't need to come up in review.

It depends on how important you think it is to optimize for future readers or maintainers of the code. I weigh that pretty heavily and a do think relatively minor style issues impose a tax once they're pervasive in a codebase.

Tooling is great, but I think there are a lot of style issues that aren't readily enforceable with linters - commenting, naming, control flow, abstraction, etc.

Post reply on HN