The Pushback Effects of Race, Ethnicity, Gender, and Age in Code Review
cacm.acm.orgIt seems that the “pushback” in this study is self-reported. It is possible that for the same code review, people from marginalized groups will self-report higher levels of pushback than people from non-marginalized groups, owing to persistent discrimination experienced elsewhere. For example, minorities posting to StackOverflow self-report[0] that it is a hostile place at greater rates than non-minorities, even though most StackOverflow posts do not reveal anything about the demographics of the poster.
In this light, it would be interesting to see a control study that focuses only on anonymized contributions to open source repositories, where one’s demographics are completely invisible. If we also see people from marginalized groups self-reporting greater pushback at similar rates, it would show that it is simply a difference in perception, not actual discriminatory behavior from the reviewer.
[0] https://stackoverflow.blog/2018/04/26/stack-overflow-isnt-ve...: “But how do we really know that too many developers experience Stack Overflow as an unwelcoming or hostile place? Well, the nice thing about problems that relate to how people feel is that finding the truth is easy. Feelings have no “technically correct.” They’re just what the feeler is telling you. When someone tells you how they feel, you can pack up your magnifying glass and clue kit, cuz that’s the answer. You’re done. And a lot of devs feel like Stack Overflow is an intimidating, unwelcoming place. We know because they tell us.”
>” It is possible that people from marginalized groups will self-report higher levels of pushback for the same code review than people from non-marginalized groups, owing to persistent discrimination experienced elsewhere.”
I think this is quite the possibility like the child who gets picked on by parents and siblings feels like when strangers critique them it’s part of the same dynamic when it isn’t.
What makes you think it was self-reported? In the linked video it says "we measured this in a few ways using our tool logs".
Code review is broken. A gate-keeper blocks progress by nitpicking every commit. They feel powerful and useful, though the opposite is true.
Everybody has one; everybody has a story. It's endemic.
I propose each time a code reviewer completes their comment they hit 'approve' and move on. Leave it to the engineer to decide what changes to embrace and which are noise. They are, after all, the expert on the code they changed.
As a reviewer, this is how I approach it. And it has been catching on my team.
As a committer. I try to make small commits. They are easier to review.- Don't block the commit. Review as soon as possible. - Focus on finding bugs. - Ask questions if you don't understand something. - Sometimes I make suggestions. - Before requesting a change I ask myself. "Is it wrong or is it just not the way I would do it".I agree, a good set of rules is helpful.
But it only takes one person to ball up the whole process. Worse if they're some self-important early hire, promoted above their skills and full of idealism. Nobody will tell them to stfu. The tools enable them, since the process won't proceed until they hit a magic button.
I've suffered under this special kind of gatekeeping autocrat. My son has on several occasions. We've both left jobs in part because of it.
Agreed. Before instituting mandatory code reviews, nobody asked if people need to be trained in code reviewing.
Nitpickers are the worst. Before someone replies with "use linters" and other such obvious things, I'd like to ensure that nitpicking often goes beyond automation. Nitpicking is imposing preferences over others. If a language or infra supports something, nitpicking one vs another is often futile in the scheme of things.
I would strongly encourage code reviewers to self reflect and see how many PRs reviewed by them actually end up in production within a day. If you don't have many, you are the problem.
In general there's no reason to think that the committing engineer is more of an expert than the reviewers. In my experience the reverse is often true.
Then the reviewer should make that clear. And often that happens.
My objection is to the gatekeeping self-appointed code hero that has some arbitrary plan. One they probably don't adhere to themselves, but insist everybody else toes the line. Don't free objects that way! Don't test for null like that! I know a better way I heard of in a blog or a meme or on a message board, and I will delay the project and burn runway to make myself feel important!
This is why competent organizations have written coding style guidelines. Everyone's code should look the same, and personal preferences shouldn't be allowed for things like freeing objects or testing for null. When the committer and reviewer can't reach consensus between themselves then they can always escalate to the program technical lead for a final decision.
Now, see, that worries me right there. Because its just something a code-review obstructionist would say. They probably wrote the guidlines too.
It doesn't matter how you do those simple things. Almost always. If a case arrises where it does matter and somebody uses the wrong one, then by all means bring it up. Otherwise, hit Approve and move on.
Enforcing consistent compliance with the guidelines starts to matter a lot in large organizations where code lifetimes are measured in decades. When you see a commit that's out of compliance on the details that's usually a sign of deeper, more serious quality problems.
Working on code bases like that isn't for everyone. Some engineers are better suited to less demanding roles where the little details don't matter as much.
Sorry, didn't expect that to sound like such an ad-hominem.
How about: the attitude that the code appearance trumps the project goals, is central to the confusion that breaks code review?