Building an Inclusive Code Review Culture • Hacker News Board

Re: Building an Inclusive Code Review Culture

#21

Post by kanox » Mon, Jul 30, 2018, 4:11 PM UTC

starbugs wrote: post #13

crunchlibrarian wrote: post #2

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 i…

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.

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.

Re: Building an Inclusive Code Review Culture

#22

Post by jelambs » Mon, Jul 30, 2018, 4:17 PM UTC

ISL wrote: post #19

aequitas wrote: post #15

"If someone has committed many crimes against the style guide in a PR, the reviewer should point them to the style guide..." Everything a computer can do trivially should not be done by humans. There are plenty of tools available to validate code to all the standards you can dream up. And most languages allow you to even automatically fix style errors or even enforce it for compilation (thank you Go). So instead of c…

> Everything a computer can do trivially should not be done by humans. Agreed, to a point. In many contexts, it is important for humans to do things themselves occasionally in order to learn how, even if a computer is generally used to do those things far more quickly.

Agreed, computers are generally way better than humans at catching many style issues. We utilize linters extensively, those enforce many of our style guide rules. However, there are some, such as the example pictured in the article, that aren’t always detectable with a linter. In these cases, we use the style guide as the source of truth to reduce the amount of personal preference that comes into code reviews.

Re: Building an Inclusive Code Review Culture

#23

Post by daveFNbuck » Mon, Jul 30, 2018, 4:19 PM UTC

nilkn wrote: post #16

daveFNbuck wrote: post #14

Earlier quoted context omitted.

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

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.

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.

Re: Building an Inclusive Code Review Culture

#24

Post by starbugs » Mon, Jul 30, 2018, 4:22 PM UTC

kanox wrote: post #21

starbugs wrote: post #13

Earlier quoted context omitted.

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.

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.

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.

Re: Building an Inclusive Code Review Culture

#25

Post by Coding_Cat » Mon, Jul 30, 2018, 4:23 PM UTC

aequitas wrote: post #15

"If someone has committed many crimes against the style guide in a PR, the reviewer should point them to the style guide..." Everything a computer can do trivially should not be done by humans. There are plenty of tools available to validate code to all the standards you can dream up. And most languages allow you to even automatically fix style errors or even enforce it for compilation (thank you Go). So instead of c…

Not all languages have good linting/autoformatting support (although luckilt they're rarer these days). For example: R.

Re: Building an Inclusive Code Review Culture

#26

Post by crdoconnor » Mon, Jul 30, 2018, 4:26 PM UTC

zhangela wrote: post #4

How do you all make sure the code review requests get load balanced across the team, as opposed to 2-3 people getting all the PR requests?

I always thought that it was ideal that the most experienced person does the bulk of code reviews and clicks the merge button even if that means that they don't write much code themselves. It's a model that, for example, works very for Linux.

I think that there's a lot of value in having somebody who has a picture in their head of the entirety of a system who can spot impedance mismatches across projects and spot germinating code smells before they cause major problems.

Re: Building an Inclusive Code Review Culture

#27

Post by nck4222 » Mon, Jul 30, 2018, 4:27 PM UTC

kanox wrote: post #21

starbugs wrote: post #13

Earlier quoted context omitted.

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.

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.

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

Re: Building an Inclusive Code Review Culture

#28

Post by UncleMeat » Mon, Jul 30, 2018, 4:30 PM UTC

starbugs wrote: post #24

kanox wrote: post #21

Earlier quoted context omitted.

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.

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.

OP is implying that this problem exists everywhere. This is simply not the case.

Re: Building an Inclusive Code Review Culture

#29

Post by starbugs » Mon, Jul 30, 2018, 4:31 PM UTC

daveFNbuck wrote: post #23

nilkn wrote: post #16

Earlier quoted context omitted.

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.

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.

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.

Re: Building an Inclusive Code Review Culture

#30

Post by aequitas » Mon, Jul 30, 2018, 4:41 PM UTC

ISL wrote: post #19

aequitas wrote: post #15

"If someone has committed many crimes against the style guide in a PR, the reviewer should point them to the style guide..." Everything a computer can do trivially should not be done by humans. There are plenty of tools available to validate code to all the standards you can dream up. And most languages allow you to even automatically fix style errors or even enforce it for compilation (thank you Go). So instead of c…

> Everything a computer can do trivially should not be done by humans. Agreed, to a point. In many contexts, it is important for humans to do things themselves occasionally in order to learn how, even if a computer is generally used to do those things far more quickly.

I agree with that if you would be learning a language. But even then, being bothered by indenting, alignment, spacing etc gets tiresome and the small amounts of time lost accumulate over the day.