On code reviews...

Posted Sep 19, 2018 4:45 UTC (Wed) by bentley (subscriber, #93468)
Parent article: Code, conflict, and conduct

> The nature of kernel development requires relatively high levels of pushback on code submissions...

It seems that people often use code reviews and technical disagreement as an example of what might stress a CoC, or cause conflict that violates it. IMO, if a person can't review code or have technical disagreements without harassing somebody or creating an exclusionary environment, they shouldn't be reviewing code or having technical discussions.

There's a big difference between "This code isn't up to our standards, here's what you can do to fix it" or "I don't have time to help you fix this code, please look here<link> for guidance" and "this code is bad (and you should feel bad)".

to post comments

On code reviews...

Posted Sep 19, 2018 7:30 UTC (Wed) by k8to (guest, #15413) [Link] (6 responses)

In my experience in corpoate coding environments, there's a *lot* of reviews that happen in the middle. Many pointlessly curt, needlessly rude reviews that don't go so far as obviously unacceptable.

I raise this because I think this is an area that needs improvement, industry-wide. The Linux kernel discussion isn't mostly full of the kind of condescension and de-valuation that is more common.

On code reviews...

Posted Sep 19, 2018 12:44 UTC (Wed) by pj (subscriber, #4506) [Link] (2 responses)

One man's "pointlessly curt, needlessly rude" is another's review done quickly while under deadline because management, while saying code reviews are required, doesn't actually want to build time into the schedule for doing them.

Everyone is fighting a secret battle you know nothing about.

On code reviews...

Posted Sep 19, 2018 16:06 UTC (Wed) by daniel (guest, #3181) [Link]

One person's insurmountable obstacle is another person's chance to show leadership.

On code reviews...

Posted Sep 19, 2018 19:53 UTC (Wed) by k8to (guest, #15413) [Link]

No, this isn't the general trend.

On code reviews...

Posted Sep 19, 2018 16:30 UTC (Wed) by bentley (subscriber, #93468) [Link] (2 responses)

I agree. I'm lucky that my current (also my first) software job is at a company that values friendliness and "playing well with others." The harshest feedback I've received is "have you considered [better idea] instead?"

OTOH I'm often hesitant to contribute to open source projects since I've seen enough examples of maintainers saying "this is stupid". I'm sure most big projects won't respond that way to a new contributor, but seen small, one-person projects reject pull requests in a needlessly curt manner.

On code reviews...

Posted Sep 20, 2018 4:03 UTC (Thu) by JdGordy (subscriber, #70103) [Link] (1 responses)

> The harshest feedback I've received is "have you considered [better idea] instead?"

Either you're one of the good devs that actually learn, or your reviewers are too soft. I try to give new hires the benefit of the doubt and give them plenty of leniecy on reviews at the beginning, and a good chunk of them actually do learn, but then there is the people who constantly need to be reminded of coding standards, and common patterns and worse, those reviews quickly go from "remind yourself of the coding standard" to "FIX", "FFS you know better", "go over the whole patch, im not wasting my time on the important bits when you cant even get formatting correct".

My understanding of Linus (as an outsider) was that he was pretty easy going with new people who honestly were trying and only went aggressive with people who (in his opinion) should have known better, and if thats accurate then I'm guilty of the same. In the corporate world this ideally ends up with being managed out but you cant really do that in the OSS world.

On code reviews...

Posted Sep 20, 2018 5:54 UTC (Thu) by Nemo_bis (guest, #88187) [Link]

The examples chosen by Noam Cohen (linked below) would seem to corroborate JdGordy's impression: in https://lkml.org/lkml/2012/12/23/75 , for instance, Linus was harsh on a person of relative power (a maintainer) to rescue the abused powerless (a bug reporter, userspace developers). Certainly this may make some power contributors uncomfortable, but it makes for a more inclusive community than projects where the non-maintainers are lumpenproletariat.

Moral leadership works better when the leader shows they're ready to admit mistakes and improve. This positive example by Linus makes me hope he will return better and stronger.

It's certainly a positive model for decisionmakers who don't admit mistakes or, even when admitting mistakes, don't ever show any sign of self-correction (e.g. https://www.techdirt.com/articles/20180914/10552840641/gu...).