5 ms·

Most code reviews I've participated in have devolved into the senior/loudest person making sure that people know who they are and what everyone's place in the h

Most code reviews I've participated in have devolved into the senior/loudest person making sure that people know who they are and what everyone's place in the hierarchy is. Even in reviews where the reviewee had some amazing code that was kind of groundbreaking in one area, still have to find something to gripe about because I have 20 years on this kid. This seems to be quite common in science/engineering, not just in code reviews. I wish I could find that company where nobody gives a shit about their place in the hierarchy and everyone isn't constantly trying to gun for other people's jobs to make their linkedin profiles look more impressive. It's been a looooong time now, still searching.

ghthor 8y ago
If you're in NYC or SF apply to foursquare. We're not perfect, but we have a nice culture that is engineer driven and is very egalitarian.
notyourwork 8y ago
My current company doesn’t have that problem (at least in my team) but I’m sure some teams do. I think this is everywhere and depends on team culture.
Bjartr 8y ago
> I wish I could find that company where nobody gives a shit about their place in the hierarchy and everyone isn't constantly trying to gun for other people's jobs to make their linkedin profiles look more impressive. It's been a looooong time now, still searching. They definitely exist (I work at such a place) so I wish you the best of luck in your search!
lolc 8y ago
Yes. There shouldn't be any reviewee except the code itself. But this is hard to get right. One of the questions I try to ask myself with any comment I write as a reviewer: "Would I accept this comment?".
- jelambs 8y ago
  Author here, I think that’s a great approach. We surface our guidelines for every PR by including a link to them in the description template. This acts as a reminder for people to be thoughtful about their review. I think the fact that our guidelines were based on input from the most senior to the most junior engineers says a lot about how much we value everyone’s opinions and perspective, not just the loudest voices.
daveFNbuck 8y ago
> still have to find something to gripe about because I have 20 years on this kid I wish I had someone with decades more experience than me taking the time to find areas where my code could improve. Regardless of their motivation, this sounds like an amazing resource that you're lucky to have.
- nilkn 8y ago
  I think their motivation still matters quite a bit. Just because someone has 20+ years of experience doesn't mean they're actually leveraging any of that for their review. If they're nitpicking something unimportant and they're only doing it for political reasons, then in that act they are contributing negative value to the team, regardless of experience.
  - daveFNbuck 8y ago
    If someone is that much more experienced than you are, it can be hard for you to tell the difference between nitpicking and something that experience has taught them is more important than you realize. Even if it is just nitpicking, that still means your code has nits to pick.
    - starbugs 8y ago
      Be sure that I can come up with nitpicking on just about anything that you write, regardless how brilliant you think your piece of code is. This just costs money, burns people, and results in a bad culture. But I am sure as long as you're not the one who is in the weaker position that's totally ok for you, because you give considerably more fks about yourself than about your company's success.
      - megaman22 8y ago
        Unless code review is a rubber-stamping process (in which case why bother? just party on production...), you're inviting criticism and improvement, so freaking out about receiving some and getting defensive or calling people toxic is... less than productive. One prolly shouldn't be a jackass as a reviewer, but if comments or questions on what you're doing get your back up, one should perhaps grow a thicker skin as a reviewee.
starbugs 8y ago
Not sure why this is being down voted. This is exactly the number one thing I have experienced happening with code reviews, especially peer reviews. This attitude often completely breaks the effectiveness of the whole process, making it a deal breaker for quick iterations and agile projects.
- kanox 8y ago
  Maybe because many people haven't experienced this? The parent poster also said this: > I wish I could find that company where nobody gives a shit about their place in the hierarchy and everyone isn't constantly trying to gun for other people's jobs to make their linkedin profiles look more impressive. This does not resonate at all.
  - starbugs 8y ago
    So "I haven't experienced this before therefore it doesn't exist. Have a downvote :)"? I kind of agree that the parent poster's tone could have been chosen with a bit more care though.
    - UncleMeat 8y ago
      OP is implying that this problem exists everywhere. This is simply not the case.
      - starbugs 8y ago
        Agreeing on that. But oh look, it's no longer being downvoted. Perhaps there's something to this after all?
  - nck4222 8y ago
    >Maybe because many people haven't experienced this? Oh god I hope not. What better way to create an echo chamber than to silence the voices of everyone who has different experiences than you...
mattlondon 8y ago
We dont have that problem where I work so it does exist. I've sort of experienced something similar myself when I've been doing reviews though - the feeling that you've got to find something to comment on. I've also been "star-struck" by getting reviewed by someone very senior or "famous" in the open source/tech world and basically blindly doing what they suggest. The problem we do have though is huge queues for qualified reviewers and getting randomly allocated a reviewer somewhere on the other side of the planet so that even if they have minor comments (e.g. trivial typos in comments) you're looking at a 48 hour turnaround to get your code checked in sometimes. I guess for the age/hierarchy thing, it could be possible to make the reviews double-blind via the tooling? I.e. reviewers dont know who they are reviewing, and the reviewee doesn't know who is reviewing them. Obviously once the code is committed then everything would become clear and transparent. Might help in larger orgs, although I dont doubt that it would be fairly easy to make an educated guess who is writing the code a lot of the time, and you'd be able to dig into git/whatever to see the details, but just hiding it in the review tool might go a long way to prevent unconscious biases. If someone has a real axe to grind is deliberately going out of their way to find out who wrote the code during a double-blind review so they can make special comments then you probably have a bigger problem.
sanderjd 8y ago
This is one failure mode that definitely exists. Another failure mode is less experienced folks who believe all their code is perfect and take criticism personally. I honestly don't know which failure mode is more common. I do know that I had the latter problem in spades when I was earlier in my career and that I'm really paranoid now about being that senior person on a power trip. But if everyone is trying really hard to both act in good faith themselves and assume good faith in their coworkers, careful reviews are the best way I'm aware of for getting great code into the tree.
reggieband 8y ago
We have a reasonable way of dealing with this - bugs found in code reviews MUST be fixed - coding style violations MUST be fixed (ideally all of these are caught by lint but not all are) - everything else is treated as optional Some code review tools even provide the ability to specify required vs. optional feedback. We now use github reviews which doesn't have this feature so we usually just add OPTIONAL to any review comment that isn't in the MUST fix category. Occasionally a reviewer and reviewee will get into discussion/disagreement over the approach a particular code change. In those cases if it isn't clearly a bug or a style violation we err on the side of the reviewee.
ironjunkie 8y ago
I have seen that in a lot of places also. There is that need for the reviewer to prove himself by nitpicking non-issues in the code.

ghthor 8y ago

If you're in NYC or SF apply to foursquare. We're not perfect, but we have a nice culture that is engineer driven and is very egalitarian.

notyourwork 8y ago

My current company doesn’t have that problem (at least in my team) but I’m sure some teams do. I think this is everywhere and depends on team culture.

Bjartr 8y ago

> I wish I could find that company where nobody gives a shit about their place in the hierarchy and everyone isn't constantly trying to gun for other people's jobs to make their linkedin profiles look more impressive. It's been a looooong time now, still searching. They definitely exist (I work at such a place) so I wish you the best of luck in your search!

lolc 8y ago

Yes. There shouldn't be any reviewee except the code itself. But this is hard to get right. One of the questions I try to ask myself with any comment I write as a reviewer: "Would I accept this comment?".

jelambs 8y ago
Author here, I think that’s a great approach. We surface our guidelines for every PR by including a link to them in the description template. This acts as a reminder for people to be thoughtful about their review. I think the fact that our guidelines were based on input from the most senior to the most junior engineers says a lot about how much we value everyone’s opinions and perspective, not just the loudest voices.

daveFNbuck 8y ago

> still have to find something to gripe about because I have 20 years on this kid I wish I had someone with decades more experience than me taking the time to find areas where my code could improve. Regardless of their motivation, this sounds like an amazing resource that you're lucky to have.

nilkn 8y ago
I think their motivation still matters quite a bit. Just because someone has 20+ years of experience doesn't mean they're actually leveraging any of that for their review. If they're nitpicking something unimportant and they're only doing it for political reasons, then in that act they are contributing negative value to the team, regardless of experience.
- daveFNbuck 8y ago
  If someone is that much more experienced than you are, it can be hard for you to tell the difference between nitpicking and something that experience has taught them is more important than you realize. Even if it is just nitpicking, that still means your code has nits to pick.
  - starbugs 8y ago
    Be sure that I can come up with nitpicking on just about anything that you write, regardless how brilliant you think your piece of code is. This just costs money, burns people, and results in a bad culture. But I am sure as long as you're not the one who is in the weaker position that's totally ok for you, because you give considerably more fks about yourself than about your company's success.
    - megaman22 8y ago
      Unless code review is a rubber-stamping process (in which case why bother? just party on production...), you're inviting criticism and improvement, so freaking out about receiving some and getting defensive or calling people toxic is... less than productive. One prolly shouldn't be a jackass as a reviewer, but if comments or questions on what you're doing get your back up, one should perhaps grow a thicker skin as a reviewee.

starbugs 8y ago

Not sure why this is being down voted. This is exactly the number one thing I have experienced happening with code reviews, especially peer reviews. This attitude often completely breaks the effectiveness of the whole process, making it a deal breaker for quick iterations and agile projects.

kanox 8y ago
Maybe because many people haven't experienced this? The parent poster also said this: > I wish I could find that company where nobody gives a shit about their place in the hierarchy and everyone isn't constantly trying to gun for other people's jobs to make their linkedin profiles look more impressive. This does not resonate at all.
- starbugs 8y ago
  So "I haven't experienced this before therefore it doesn't exist. Have a downvote :)"? I kind of agree that the parent poster's tone could have been chosen with a bit more care though.
  - UncleMeat 8y ago
    OP is implying that this problem exists everywhere. This is simply not the case.
    - starbugs 8y ago
      Agreeing on that. But oh look, it's no longer being downvoted. Perhaps there's something to this after all?
- nck4222 8y ago
  >Maybe because many people haven't experienced this? Oh god I hope not. What better way to create an echo chamber than to silence the voices of everyone who has different experiences than you...

mattlondon 8y ago

We dont have that problem where I work so it does exist. I've sort of experienced something similar myself when I've been doing reviews though - the feeling that you've got to find something to comment on. I've also been "star-struck" by getting reviewed by someone very senior or "famous" in the open source/tech world and basically blindly doing what they suggest. The problem we do have though is huge queues for qualified reviewers and getting randomly allocated a reviewer somewhere on the other side of the planet so that even if they have minor comments (e.g. trivial typos in comments) you're looking at a 48 hour turnaround to get your code checked in sometimes. I guess for the age/hierarchy thing, it could be possible to make the reviews double-blind via the tooling? I.e. reviewers dont know who they are reviewing, and the reviewee doesn't know who is reviewing them. Obviously once the code is committed then everything would become clear and transparent. Might help in larger orgs, although I dont doubt that it would be fairly easy to make an educated guess who is writing the code a lot of the time, and you'd be able to dig into git/whatever to see the details, but just hiding it in the review tool might go a long way to prevent unconscious biases. If someone has a real axe to grind is deliberately going out of their way to find out who wrote the code during a double-blind review so they can make special comments then you probably have a bigger problem.

sanderjd 8y ago

This is one failure mode that definitely exists. Another failure mode is less experienced folks who believe all their code is perfect and take criticism personally. I honestly don't know which failure mode is more common. I do know that I had the latter problem in spades when I was earlier in my career and that I'm really paranoid now about being that senior person on a power trip. But if everyone is trying really hard to both act in good faith themselves and assume good faith in their coworkers, careful reviews are the best way I'm aware of for getting great code into the tree.

reggieband 8y ago

We have a reasonable way of dealing with this - bugs found in code reviews MUST be fixed - coding style violations MUST be fixed (ideally all of these are caught by lint but not all are) - everything else is treated as optional Some code review tools even provide the ability to specify required vs. optional feedback. We now use github reviews which doesn't have this feature so we usually just add OPTIONAL to any review comment that isn't in the MUST fix category. Occasionally a reviewer and reviewee will get into discussion/disagreement over the approach a particular code change. In those cases if it isn't clearly a bug or a style violation we err on the side of the reviewee.

ironjunkie 8y ago

I have seen that in a lot of places also. There is that need for the reviewer to prove himself by nitpicking non-issues in the code.