7 ms·

I feel like this is often overlooked. One morning, I did a code review before having any coffee. I basically used the latter two throughout the whole thing. I

I feel like this is often overlooked. One morning, I did a code review before having any coffee. I basically used the latter two throughout the whole thing. I came into the office to find my coworker literally crying. Later in the day, someone else said it was the best code review they’d ever read… and asked me to come to their team… There was so much drama from that code review. That code got merged as-is to appease the author without a single one of my statements addressed (including security considerations), and no one else would review it. My manager was in a tight spot, felt sorry for him. I was literally just my pre-coffee blunt self. I usually use the first few versions in your list, which is probably why it hit my coworker so hard. We chatted and made up, but our relationship was weird after that. So, I’d say, just be consistent. Changing tones unexpectedly might affect your coworkers and politics in interesting ways that you might not expect.

muzani 4y ago
Yeah, I think code reviews strike deep for whatever reason. I don't think being blunt works here even if you can be blunt elsewhere.
ianmcgowan 4y ago
You sound like a Dutch person. Working with different cultures I've had to adapt to varying levels of directness but like to think it's helped me become more up-front. I've learned to appreciate people that say what they're thinking, so you can avoid the 2nd and 3rd order analysis about intent and thoughts about how other people will receive things.
- comprev 4y ago
  I found the Dutch directness a breath of fresh air when working in NL.
- withinboredom 4y ago
  Is it ironic I moved to the Netherlands and feel right at home here? (this story is from when I lived in the US)
dominotw 4y ago
Is this person working on his own. Why can't people pair and give each other at the moment feedback vs doing code review. Person is already invested in his code by PR review time and doesn't really want to do PR ping pong for their task. What if you come back with more comments after they address your comments. Will they ever get to finish their task or forever be hostage to you subjective comments. Most of it is usually subjective too. you had to explicitly mention 'security issues' which imply that you have a feeling that most of your pr comments were your subjective interpretation . People are more open to suggestions during pair programming because people don't deliver feedback in cutting way ( with or without coffee) and its easy for get a common understanding and move forward.
- withinboredom 4y ago
  > Why can't people pair and give each other at the moment feedback vs doing code review. That was something we discussed at various points. We never tried it when I worked there. > you had to explicitly mention 'security issues' which imply that you have a feeling that most of your pr comments were your subjective interpretation . Aren't almost all code review comments? I can't think of a single one except ones that point out literal bugs. Most are "do it this way because I think it will be more maintainable" or "do it this way because that is how we do it here." In this case, I mentioned "security issues" because they were literal bugs that introduced new attack surfaces that didn't previously exist. When I read comments on my code, I make sure to read it without any inflection. Some people have really bizarre code review styles, and some are totally straightforward. Some people are lenient on whether or not there actually needs to be a change, while some people will not accept code until changes are made, no matter how little the change request is. Knowing your reviewer is about as important as how you review code, which is why I suggested being consistent. If you're always straightforward, and then suddenly not, the reviewee is going to wonder if something is going on. Vice-versa, if you're suddenly straightforward, people are going to be offended or feel like you are taking something out on them. In my case, reviewing code within 3 minutes of waking up was a terrible idea and I have never done it since because I'm "grumpy-mc-grumpy-pants" for at least 10-15 minutes after waking up and having some coffee. I feel like a lot of the conversation in this thread is about how to review code, but the relationship between the reviewee and the reviewer is more important than any of that and how you review code will affect that relationship whether you want it to or not.
59nadir 4y ago
> I was literally just my pre-coffee blunt self. I have a co-worker who's socially retarded as well and I can say that the vast majority of his comments that tend to rub people the wrong way are just badly phrased versions of legitimate opinions (that he sometimes should just keep to himself because no one asked). He's a better reviewer than you seem to be, though; probably because he can take the time to type out better versions of things he'd normally just blurt out. Edited to add: It's a massive chore to have a teammate that causes stuff like this and I don't envy your teammates or lead (even worse if you're the lead, obviously).
- withinboredom 4y ago
  Considering this story is over a decade old, and I was in my late teens ... I'd hope that I've grown quite a bit even though you're using the present tense like you know me.
  - fknorangesite 4y ago
    The way you told the story it comes off as much more recent. > I'd hope that I've grown quite a bit I hope so too, because "I hadn't had my coffee yet har har" is a pretty thin excuse.
    - withinboredom 4y ago
      > because "I hadn't had my coffee yet har har" is a pretty thin excuse. I'm pretty blunt/rude in the mornings, within half an hour of waking up, even these days. Back then, I did not know that about myself -- or rather, just how rude I came across. Most people I interact with never see me like that, but it generally takes me 10-15 minutes to get some coffee going. Hence why I said "before coffee" since I assume nearly everyone is a little weird within the first 30 minutes of waking up, which also tends to be before a morning coffee. People who wake up "ready to go" seem to be pretty rare. Granted, my number of people I've had the opportunity to be around when they wake up is only a few hundred people (basic training, etc).
      - 4y ago
mythrwy 4y ago
Wow not excusing hurting people's feelings, but if you are literally crying you have become too identified with your code and possibly your job. Or else, you just cry too easily and that's a problem when working with others also.
tayo42 4y ago
Crying? Did you leave some Linus Torvalds level feedback, like tell them to find a new career or something?
- noisy_boy 4y ago
  That's not level==Linus; for that you'll have to tell something like: "You wrote the code, and you seem to be unable to admit that your code was buggy. It's not a compiler bug. It's your bug. Stand up like a man, instead of trying to flail around and blame anything else but yourself."
  - wutbrodo 4y ago
    Needs more profanity to be accurate
rjdamore 4y ago
People are sensitive. Directness is underappreciated. Inability to take critique is killing code quality. Try going to an art school. Critique is how we get better. Check your ego at the door and you'll go far as a programmer
- menaerus 4y ago
  I'm one of the likes who appreciates directness over subtle critique. I hate the latter but I also found out that it seems to work better with most of the people.
- LAC-Tech 4y ago
  Try going to an art school. Critique is how we get better. I've seen modern art - it's not working.
- agentultra 4y ago
  They have courses on constructive critique in art school! I think software developers would do well to take such a course.
gwbas1c 4y ago
I'm curious, what happened in the long run? Was this developer generally unprofessionally sensitive? What happened to your manager?
- withinboredom 4y ago
  I left the company within the next few months, the manager is still doing well. I haven’t kept in touch with the other dev.
  - gwbas1c 4y ago
    Good move. I had a similar overly-sensitive colleague. (She was support, and would get flustered at my short responses like "please file a ticket" or "logs.") She left the day after I met with her and explained common sense. (Customers expect you to know how to use the product; don't bypass the ticketing system by emailing me directly; follow the documented process to escalate to engineering; don't just copy and paste customer emails without doing your own investigation...) I was having a lot of trouble with getting my manager to intervene with her, so in this case her departure was a godsend. We ended up firing my manager a little later, and things ran so smoothly afterwards that I realized he was the real source of the problems.

muzani 4y ago

Yeah, I think code reviews strike deep for whatever reason. I don't think being blunt works here even if you can be blunt elsewhere.

ianmcgowan 4y ago

You sound like a Dutch person. Working with different cultures I've had to adapt to varying levels of directness but like to think it's helped me become more up-front. I've learned to appreciate people that say what they're thinking, so you can avoid the 2nd and 3rd order analysis about intent and thoughts about how other people will receive things.

comprev 4y ago
I found the Dutch directness a breath of fresh air when working in NL.
withinboredom 4y ago
Is it ironic I moved to the Netherlands and feel right at home here? (this story is from when I lived in the US)

dominotw 4y ago

Is this person working on his own. Why can't people pair and give each other at the moment feedback vs doing code review. Person is already invested in his code by PR review time and doesn't really want to do PR ping pong for their task. What if you come back with more comments after they address your comments. Will they ever get to finish their task or forever be hostage to you subjective comments. Most of it is usually subjective too. you had to explicitly mention 'security issues' which imply that you have a feeling that most of your pr comments were your subjective interpretation . People are more open to suggestions during pair programming because people don't deliver feedback in cutting way ( with or without coffee) and its easy for get a common understanding and move forward.

withinboredom 4y ago
> Why can't people pair and give each other at the moment feedback vs doing code review. That was something we discussed at various points. We never tried it when I worked there. > you had to explicitly mention 'security issues' which imply that you have a feeling that most of your pr comments were your subjective interpretation . Aren't almost all code review comments? I can't think of a single one except ones that point out literal bugs. Most are "do it this way because I think it will be more maintainable" or "do it this way because that is how we do it here." In this case, I mentioned "security issues" because they were literal bugs that introduced new attack surfaces that didn't previously exist. When I read comments on my code, I make sure to read it without any inflection. Some people have really bizarre code review styles, and some are totally straightforward. Some people are lenient on whether or not there actually needs to be a change, while some people will not accept code until changes are made, no matter how little the change request is. Knowing your reviewer is about as important as how you review code, which is why I suggested being consistent. If you're always straightforward, and then suddenly not, the reviewee is going to wonder if something is going on. Vice-versa, if you're suddenly straightforward, people are going to be offended or feel like you are taking something out on them. In my case, reviewing code within 3 minutes of waking up was a terrible idea and I have never done it since because I'm "grumpy-mc-grumpy-pants" for at least 10-15 minutes after waking up and having some coffee. I feel like a lot of the conversation in this thread is about how to review code, but the relationship between the reviewee and the reviewer is more important than any of that and how you review code will affect that relationship whether you want it to or not.

59nadir 4y ago

> I was literally just my pre-coffee blunt self. I have a co-worker who's socially retarded as well and I can say that the vast majority of his comments that tend to rub people the wrong way are just badly phrased versions of legitimate opinions (that he sometimes should just keep to himself because no one asked). He's a better reviewer than you seem to be, though; probably because he can take the time to type out better versions of things he'd normally just blurt out. Edited to add: It's a massive chore to have a teammate that causes stuff like this and I don't envy your teammates or lead (even worse if you're the lead, obviously).

withinboredom 4y ago
Considering this story is over a decade old, and I was in my late teens ... I'd hope that I've grown quite a bit even though you're using the present tense like you know me.
- fknorangesite 4y ago
  The way you told the story it comes off as much more recent. > I'd hope that I've grown quite a bit I hope so too, because "I hadn't had my coffee yet har har" is a pretty thin excuse.
  - withinboredom 4y ago
    > because "I hadn't had my coffee yet har har" is a pretty thin excuse. I'm pretty blunt/rude in the mornings, within half an hour of waking up, even these days. Back then, I did not know that about myself -- or rather, just how rude I came across. Most people I interact with never see me like that, but it generally takes me 10-15 minutes to get some coffee going. Hence why I said "before coffee" since I assume nearly everyone is a little weird within the first 30 minutes of waking up, which also tends to be before a morning coffee. People who wake up "ready to go" seem to be pretty rare. Granted, my number of people I've had the opportunity to be around when they wake up is only a few hundred people (basic training, etc).
    - 4y ago

mythrwy 4y ago

Wow not excusing hurting people's feelings, but if you are literally crying you have become too identified with your code and possibly your job. Or else, you just cry too easily and that's a problem when working with others also.

tayo42 4y ago

Crying? Did you leave some Linus Torvalds level feedback, like tell them to find a new career or something?

noisy_boy 4y ago
That's not level==Linus; for that you'll have to tell something like: "You wrote the code, and you seem to be unable to admit that your code was buggy. It's not a compiler bug. It's your bug. Stand up like a man, instead of trying to flail around and blame anything else but yourself."
- wutbrodo 4y ago
  Needs more profanity to be accurate

rjdamore 4y ago

People are sensitive. Directness is underappreciated. Inability to take critique is killing code quality. Try going to an art school. Critique is how we get better. Check your ego at the door and you'll go far as a programmer

menaerus 4y ago
I'm one of the likes who appreciates directness over subtle critique. I hate the latter but I also found out that it seems to work better with most of the people.
LAC-Tech 4y ago
Try going to an art school. Critique is how we get better. I've seen modern art - it's not working.
agentultra 4y ago
They have courses on constructive critique in art school! I think software developers would do well to take such a course.

gwbas1c 4y ago

I'm curious, what happened in the long run? Was this developer generally unprofessionally sensitive? What happened to your manager?

withinboredom 4y ago
I left the company within the next few months, the manager is still doing well. I haven’t kept in touch with the other dev.
- gwbas1c 4y ago
  Good move. I had a similar overly-sensitive colleague. (She was support, and would get flustered at my short responses like "please file a ticket" or "logs.") She left the day after I met with her and explained common sense. (Customers expect you to know how to use the product; don't bypass the ticketing system by emailing me directly; follow the documented process to escalate to engineering; don't just copy and paste customer emails without doing your own investigation...) I was having a lot of trouble with getting my manager to intervene with her, so in this case her departure was a godsend. We ended up firing my manager a little later, and things ran so smoothly afterwards that I realized he was the real source of the problems.