Live data from Hacker News

Building an Inclusive Code Review Culture

blog.plaid.com

61–70 of 81 posts

Re: Building an Inclusive Code Review Culture

#61
post #50

Earlier quoted context omitted.

This is a discussion to be had before coding starts on a project. Do you care about style? If yes, choose a language that cares about style.

Not always possible if the requirements of the project dictate the language.

Build that tool yourself if one doesn't exist yet. The whole community will thank you for it.

Re: Building an Inclusive Code Review Culture

#62

The real problem here are people with bad attitude and poor communication skills. Bullying other people is a punishable offense in real life, and I don't see why code reviews should be any different. Have some basic rules regarding communication. Issue a public warning for first-time offenders. Give them the boot if they do it again. There, problem solved.

@tdsamardzhiev What sorts of "bullying" have you seen in code reviews before? Having some anti-pattern examples could be useful to codify into a developer guid.

Linus Torvalds comes to mind... Although somehow the tech community seems to think his juvenile rants are supremely brave and honest.

Re: Building an Inclusive Code Review Culture

#63

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…

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.

Re: Building an Inclusive Code Review Culture

#64
post #27
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.

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

I didn't downvote the comment, but maybe some people think it lacks perspective and concocts theories for why criticism of their code is actually the fault of reviewers rather than taking it as a learning experience. But I didn't downvote it because who knows, maybe the commenter has actually had a bunch of bad faith reviewers.

Re: Building an Inclusive Code Review Culture

#65

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

https://cran.r-project.org/web/packages/formatR/index.html

Re: Building an Inclusive Code Review Culture

#66

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 the word "gripe" was meant to imply criticism that isn't constructive and has no actual value in improving the end product. Let's say you send code out for review that is just about perfect how it is. The reviewer is faced with a choice. If they don't suggest any changes, they may feel it gives the impression that you are just good a coder as they are, that their input isn't needed, or that they didn't bother…

[deleted]

Re: Building an Inclusive Code Review Culture

#67

Earlier quoted context omitted.

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

https://cran.r-project.org/web/packages/formatR/index.html

We looked at it for our project, but it didn't work for us. Too little configuration & it made sections of code completely unreadable.

I miss clang-format...

Re: Building an Inclusive Code Review Culture

#68

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.

I agree, this is a very good point being raised. Code reviews should be geared towards the success of the team, not people's egos.

We recently instituted a code review process at my company. We're small, 4 engineers on the core system, plus our CTO. All 5 of us review every commit now. Personally, I take issue with any bug that makes it into production, especially if its mine. We inheritted a code base that was written by cheap off shore contractors and it shows. We're working on cleaning it up and improving it, but it's slow going with a small team and tons of new requirements. I personally just committed changes to +600 files...we're still reviewing after a week.

Re: Building an Inclusive Code Review Culture

#69

Earlier quoted context omitted.

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.

I don't believe this. Every time I've seen subtle problems that experience taught me were more important than first appeared, I've been able to explain why . And so I put that explanation in the review.

Usually people complain to me about nitpicking for small things I thought should have been obvious and didn't require explanation. When you point out a word in a comment is misspelled, it shouldn't require an explanation about why proper spelling is important for readability and clear communication.

Re: Building an Inclusive Code Review Culture

#70
post #40

Earlier quoted context omitted.

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.

Code reviews are generally public and visible to the entire team and often even to other teams. While it's true that the reviewee may not be able to distinguish the two cases being considered, there may be other developers on the team or in the company who witness the review and can make the distinction. In fact, this can cause the "toxic reviewing" practice to spread to other experienced senior developers, who may s…

I've never been in a work environment where senior developers are so insecure about their seniority. How can a junior developer tell the difference between this scenario and one where senior developers are making good comments on code reviews that junior developers don't see the value of?
Post reply on HN