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

11–20 of 164 posts

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

#11

Post by abnry »

futureastronaut wrote: post #5

It bothers me that the conclusion doesn't escape the framework of code reviews as places of conflict, and that the author feels guilty for his claimed competence. > No big deal if the code’s not good, I can fix it myself it I need to. A mentor can't be passive like that. Don't leave 200 comments like you used to, but distill them into something smaller, more cogent and digestible. More importantly, drop the idea of "…

The guy appears to come to the realization that he causes more conflict than necessary.

The purpose of code reviews is to get quality code. Quality code makes the project work. But why do we work on projects to begin with? To benefit people.

This guy is realizing the conflict between the two goals of getting quality code but also serving people.

People need criticism to improve, but it needs to be constructive, otherwise the fragile human psyche inside of every person may not recover.

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

#12

Post by ErrantX »

Good developers rip bad code the shreds.

Great developers see the opportunity to grow someone.

The former shows ownership of the code, but is generally bad for a business.

The latter understands that a successful business is more than simply his competence.

Reaching this level of realisation is critical for being a good lead; I interview loads of clearly solid programmers who havent figured this out, and sadly price themselves out of the market (I blame their previous managers for missing their growth opportunity)

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

#13

Post by aristophenes »

"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 everything is good enough and I can just approve it. And I won't comment unless I am convinced it is important.

I don't hate the coder, or the process that requires code reviews. I know code reviews are very important for many different reasons, so I don't shirk them, but I have become afraid of being an expert on a large team as I will have to do more of them. Not sure what is wrong, I love coding. Reviewing good code isn't so bad, reviewing bad code takes all the joy out of coding for me. If it's bad enough, I know that I need to make a lot of comments, and I'll need to make some more when it comes back with changes. I think we are just wired differently.

EDIT After further thought, I think a fundamental difference is I really want to see my coworkers succeed, and don't enjoy their failure. Also I dislike all forms of toil. But I think maybe part of the authors problem is not able to comprehend that some people can be nice like that, genuinely. In fact, I assume my coworkers want me to succeed as well, and I really think most of the time they do. But I am glad he decided to start acting that way, which must be especially difficult if you don't believe others will treat you the same way.

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

#14

Post by wccrawford »

>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.

Are you kidding? Call me a liar if you want, but I don't feel that way when doing code reviews, and never have.

Yes, when I was a teenager I wielded my knowledge like a titan. The meager amount of knowledge I had.

But as a professional, I have never once been mean-spirited in a code review. Everything I send back is to keep the code clean and maintainable and to help the developer improve.

Not everyone feels the need to be "right" all the time. And by that, I mean being perceived as correct, even if they aren't. For me, that was a function of actual skill.

The better I got at programming, the less I needed everyone else to acknowledge how good I was.

Even better, that applied to the rest of my life, too. I haven't felt the need to show off my intelligence or skills in a long, long time. I simply do my thing and if/when someone notices, it's a great feeling. If they don't, I still enjoyed doing it well.

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

#15

Post by ErrantX »

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…

Step number 1 for any dev joining my org; learn the difference between "how I would have done it" and genuine optimisation/bugs.

Its probably too carte blanche; but ive reached the point where I will refuse to rule on codestyle issues (general answer; whoever did it first sets the style)

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

#16

Post by tmd83 »

I couldn't really read it fully kind of in a run. But it seems like that the author isn't just talking about the language but that commenting on every bad issue of the code is bad? I don't agree with that.

I have a done a lot of review not as much lately but my principle is pretty simple - Can this be made better to the best of my understanding and knowledge. I will give it the same attention as I will my code. When my first version of a code or the POC is done I will think of what can be improved upon with the goal of the final version to be cleaner, faster and more robust if possible. My job is to help the author and the company to make sure together we get the best version possible merged of course with an understanding of cost-benefit.

The language is of course important but not catching issues on every line if that's what I find. I will appreciate if someone does the same thing for me. At one point I have reviewed more than 50% of the codes in my company and I know how hard it can be to do so with full care so I take it as a favor when somebody does a good review on my code finds mistakes or potential improvement. I have had way junior developers giving me feedback ranging from better names to serious bugs. I have also always explained why a certain idea should be explored or might be better with the author regardless of how junior they might be. I have always liked the developers best who leaves the ego out of it both a reviewer and author of a certain piece of code. Pride and Ego I find are unrelated. I take pride in giving my best both when writing a code or reviewing one all the while knowing not just that there are other developers much better than me but even someone who is on average worse than me can still find potential improvements in my code. I like to think that I can take any comment on its merit and not my perception of the person.

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

#17

Post by ereyes01 »

Code reviews aren't supposed to be humiliating. This article does a lot to illustrate how a negative attitude in review comments can be counter-productive, but the conclusion feels like code reviews that point out dozens of flaws in the change are bad.

I disagree that pointing out every flaw in a code review is a bad thing to do. I believe that these could be positive experiences with the right attitude from both the reviewer, the submitter, and the team's management. A few things that really bugged me while reading this:

- The reviewer feels like pointing out a flaw is an adversarial zero-sum sort of action that affirms his superiority over the submitter.

- The submitter will feel bad about each feedback given (at least in the POV of the author). If the feedback is mean-spirited, then sure, but if it's constructive, then this shouldn't be the case.

- His team fired the developer who received "too much" feedback. I'm sure there's missing context here, but if that's the main reason, then this is pretty messed up.

Finally, his conclusion to not submit the review really bugged me. I do agree that no review is better than a mean-spirited destructuve review, but IMO this is still a failure on the part of the reviewer. He should write the review in a constructive manner and work with the submitter to hash out the issues. If it's an argument, then fine, work through it... but teammates should help each other grow and be better.

In this case, by not submitting honest and comprehensive reviews, I believe his team and his product suffers in the following ways:

- The reviewer has to waste his time "cleaning up" the code later on.

- The submitter loses out on the knowledge transfer that takes place during a good code review.

- The product potentially has unfixed bugs.

- A fear of conflict is further instilled in the team, and criticism now becomes off-limits because it is considered destructive due to the negative attitudes of everyone involved. This further hurts product quality, and the result is that everyone codes in isolation out of their ivory towers instead of collaborating on a product together, among many other detrimental effects.

I think the solution at least begins with:

- A better team commitment to personal growth, and a sense of responsibility on the part of everyone to help out their teammates in this respect.

- Clear team guidelines on lint/style standards to eliminate the need to argue about it.

- An attitude that feedback is about the code and NOT about the person submitting it. Everyone should feel like an owner of the code, and all discussion should be about the code and how to improve it.

- Comprehensive code reviews. Note everything you find. Maybe call an in-person review if you feel like there's too much to comment on.

edit: typos

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

#18

Post by jaggederest »

The secret to good code review is addressing the code, not the person. Ask questions and make suggestions, rather than being critical of the other person.

Instead of "Why would you ever do it this way?", try "I think I would probably do this like (example)"

Also, be liberal with complementary reviews, as well. I see a lot of pull requests where the solid, workday, well written code is just passed over, or given a ":+1:", even though it's definitely worthy of praise.

You wrote a new class extracted from an old confusing method, it has good tests, and the feature is working as requested? A triumph.

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

#20

Post by cortesoft »

metabagel wrote: post #7

I enjoyed the post. My feeling is that criticism, no matter what the domain, should be relatively dispassionate (neutral in tone), and come from a desire to help and with a dose of humility. Of course, I sometimes fall far short of that ideal.

I actually feel that criticism should be empathetic rather than dispassionate... it should be worded to show that you understand the person is a competent professional who was working in good faith, but might have missed something or made a mistake.

This is why I try to word reviews as questions, not statements of facts:

"Is this supposed to be reversed? I think it might not do what you want it to"

"Having a bit of trouble following this logic.. is there a way to make it more clear to future readers?"

"I might be missing something, but I don't see how this code can be reached?"

Don't give the review in such a way that implies you know everything and the other person was wrong. It should be a dialog, and be based on the assumption that we don't yet know the right answer and are working together to figure it out.

Post reply on HN