Building an Inclusive Code Review Culture • Hacker News Board

Re: Building an Inclusive Code Review Culture

#71

Post by squiggleblaz » Tue, Jul 31, 2018, 1:34 AM UTC

quxbar wrote: post #37

mnarayan01 wrote: post #35

Earlier quoted context omitted.

if a = foo() versus: if (a = foo()) Sometimes idiomatic patterns vary based on things which are not amiable to mechanical correction.

The trick is to ban even the correct but (to a human) ambiguous patterns. In this case, I wouldn't let you do assignment on the same line as the conditional.

With an `if` it's probably superfluous and therefore bannable without cost, but sometimes it can make a `while` loop so much more concise - since the alternative would be to write the same code before the loop and at the bottom of the loop. It is the cost of inexpressive languages.

Re: Building an Inclusive Code Review Culture

#72

Post by brian-armstrong » Tue, Jul 31, 2018, 1:59 AM UTC

Not focusing on style violations is foolish. Imagine someone submitted an English paper littered with grammatical errors and typos. It would be difficult to find the intended meaning and bigger picture. Style violations are the same, they have to be fixed before a deeper review can even take place.

Re: Building an Inclusive Code Review Culture

#73

Post by arvinsim » Tue, Jul 31, 2018, 2:24 AM 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…

All of my formatting issues disappeared when I adopted prettier for my Javascript projects.

I don't have to worry about bikeshedding/yak shaving anymore and now can focus on more important problems.

Re: Building an Inclusive Code Review Culture

#74

Post by KKKKkkkk1 » Tue, Jul 31, 2018, 3:07 AM UTC

How common is it to get unsolicited reviews on your PRs? In my team, I have some colleagues who will routinely pipe up when they see a PR being uploaded by an "outsider" and will write a litany of complaints in order to block progress and establish their dominance. (I should mention that these colleagues work in a branch office, so this is their only way to assert themselves.)

Re: Building an Inclusive Code Review Culture

#75

Post by tdsamardzhiev » Tue, Jul 31, 2018, 4:02 AM UTC

sqrt17 wrote: post #43

tdsamardzhiev wrote: post #32

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.

You sound like you didn't read the article. There's nothing about political correctness or try-hard inclusivity in that article but many (or indeed all) the things that common sense would dictate as a base of working code reviews.

No, I didn't read the article. But I did read the entire comment section, if that counts :)

Re: Building an Inclusive Code Review Culture

#76

Post by knocte » Tue, Jul 31, 2018, 4:49 AM UTC

KKKKkkkk1 wrote: post #74

How common is it to get unsolicited reviews on your PRs? In my team, I have some colleagues who will routinely pipe up when they see a PR being uploaded by an "outsider" and will write a litany of complaints in order to block progress and establish their dominance. (I should mention that these colleagues work in a branch office, so this is their only way to assert themselves.)

> in order to block progress

This is your subjective interpretation of the facts. Learn how they complain (in the end, they only can complain about technical facts: learn them, and be correct next time), and you will not receive complaints anymore.

I'm guessing you're new to code reviews right?

Re: Building an Inclusive Code Review Culture

#77

Post by galkk » Tue, Jul 31, 2018, 4:53 AM UTC

Coding_Cat wrote: post #67

zeckalpha wrote: post #65

Earlier quoted context omitted.

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

Google's internal Java formatter doesn't (didn't) have _any_ configuration. And, as I start to believe, it's good

Re: Building an Inclusive Code Review Culture

#78

Post by [deleted] » Tue, Jul 31, 2018, 5:00 AM UTC

knocte wrote: post #76

KKKKkkkk1 wrote: post #74

How common is it to get unsolicited reviews on your PRs? In my team, I have some colleagues who will routinely pipe up when they see a PR being uploaded by an "outsider" and will write a litany of complaints in order to block progress and establish their dominance. (I should mention that these colleagues work in a branch office, so this is their only way to assert themselves.)

> in order to block progress This is your subjective interpretation of the facts. Learn how they complain (in the end, they only can complain about technical facts: learn them, and be correct next time), and you will not receive complaints anymore. I'm guessing you're new to code reviews right?

[deleted]

Re: Building an Inclusive Code Review Culture

#79

Post by KKKKkkkk1 » Tue, Jul 31, 2018, 5:13 AM UTC

nilkn wrote: post #57

Drdrdrq wrote: post #54

Earlier quoted context omitted.

Interesting - I thought you would end the argument differently. In my experience additional visibility helps curb the unwanted behaviour because other senior devs step in and help fight it - but I guess that depends on the general culture in the company. If that doesn't happen you have bigger problems anyway.

I do believe that code review culture ends up being a reflection of general company culture. In a healthy culture, visibility tends to spread the positive part of what's observable, whereas in a toxic culture visibility tends to spread the negative and political parts of what's observable. And there's definitely a circularity here. In a healthy culture, code reviews tend not to be overly political in the first place,…

As an engineer, how do you address this sort of mutually-reinforcing jerkery? I expect that complaining to the manager will not be effective because the culprits are not explicitly hostile.

Re: Building an Inclusive Code Review Culture

#80

Post by Drdrdrq » Tue, Jul 31, 2018, 6:51 AM UTC

KKKKkkkk1 wrote: post #79

nilkn wrote: post #57

Earlier quoted context omitted.

I do believe that code review culture ends up being a reflection of general company culture. In a healthy culture, visibility tends to spread the positive part of what's observable, whereas in a toxic culture visibility tends to spread the negative and political parts of what's observable. And there's definitely a circularity here. In a healthy culture, code reviews tend not to be overly political in the first place,…

As an engineer, how do you address this sort of mutually-reinforcing jerkery? I expect that complaining to the manager will not be effective because the culprits are not explicitly hostile.

Easiest way? Complain, look for another job, quit. There is no way you can change the culture from bottom up. Best you can achieve is make a bubble with your close coworkers where you cover each other - but even that is temporary and can be broken by management anytime. Sorry. But there are many companies with better culture out there...