Ah! The resounding NAK! https://lore.kernel.org/lkml/CAHk-=whFZoap+DBTYvJx6ohqPwn11P...
My take away: jeffxu@google.com has to go back to rethink the whole thing.
Good idea, execution, not so, yet.
My take away: jeffxu@google.com has to go back to rethink the whole thing.
Good idea, execution, not so, yet.
It is your job to do this. You seem to have taken the attitude that you will give the Chrome team exactly what they asked for instead of trying to understand their requirements and give them what they need. mseal() is a clone of mimmutable(2), but with an extremely over-complicated API based upon dubious arguments.He never, ever, once, demeans the person who submitted the patch or shows a pattern of abuse to that person -- if you look up the definition of "abusive" you'll see it says (emphasis mine): "extremely offensive AND insulting". While the text may be offensive (to some, to my ex-military eyes, this is being nice), he never once insulted the author.
Sucks to be on the receiving end, but in the end its better for everyone. Wish our developers could work like this.
How is it ok to berate a contribution like that? Is it encouraging more people to contribute? Is it creating a positive environment? Let's park that for now, is it helping the technical discussion at all? It's just power tripping on the gatekeeper status.
All of this could be said in a technical, professional manner. Many many great programmers do it everyday, sorry Linus, nobody is a God. People are not anybody's toilet paper.
…was really all it needed. The wall of text is more offensive than the insulting tone.
But it's also not Torvald's job to tutor here or be a mentor. He has limited time in the day. And the company doing the proposal here is Google. They probably should have come in with a more-better-finished product.
The consequences to design errors in kernel syscalls are rather intense.
So he spends it dunking on a proposal with snarky prose instead of a concise review without bashing the person trying to contribute. How efficient.
Would you rather get a code review that is full of vague euphemisms and tries not to hurt your feelings, and then spend months wasting time on a dead end approach because you thought minor tweaks would work?
I'd much rather get one message that says where the bar for merging is so I could fix my code and get it merged. I absolutely wouldn't care if the latter was a bit sweary. I've worked at a place where all communication must be in professional tone and polite, and you cannot give valid technical feedback that might hurt someone's feelings.
The verbal communications standards were weaponized by sycophants, who dominated middle management. It was the most passive-aggressive and hostile work environment (as in legal liability) I've ever encountered.
> Mark Twain who is often connected to this saying did not use it according to the best available research, but one of his tangentially related quotations is given later for your entertainment.
> In 1871 Mark Twain wrote a letter to a friend that included a remark about the length of his note. Twain’s comment did not really match the quotation under investigation but it is related to the general theme:[ref] 1871 June 15, Letter from Mark Twain to James Redpath, Elmira, New York, UCCL 00617 (Union Catalog of Clemens Letters), Mark Twain Project Online. (Accessed marktwainproject.org on 2012 April 24) link[/ref]
quoting Twain:
>> You’ll have to excuse my lengthiness—the reason I dread writing letters is because I am so apt to get to slinging wisdom & forget to let up. Thus much precious time is lost.
rm "probably"
The PR submitter represents a software giant, love 'em or hate 'em.
It is an understatement to say that the Linux kernel is an important, global-scale software project. A small number of busy people maintain the kernel's design. This is one project where crap should not be submitted.
Whether or not we like the company, a software giant like Google should a) understand the scale of the project and the importance of software design, and b) manifestly understand the rudiments of code quality.
You can see it in the translations of having 3 flags for the same data. Somewhere, the distinction was made between what the user is asking for, and what the system works with. It is considered a good thing to have that level of abstraction in many large companies. Heck, many small companies go for that.
> First off, the simple stuff: the commit messages are worthless. Having
check seal for mmap(2)
That invites pointless discussion when most of the points in Linus review are a clear "hell no". Quite sure the last thing someone in Linus position wants is never ending discussions of already rejected ideas, it only wastes time on both sides.
There is no need for such language. Other commenter here shown the whole "review" could be just one or two sentences.
That to me shows someone has not been going to therapy.
Because that's what you're seeing here.
It's a passionate founder with an abuse problem doing something that doesn't require abuse but being abusive anyway and, because they're too important, no one can easily prevent it.
First, if you fly off the handle every time somebody pushes some bad code, you might want to find some help, because it's going to happen a lot. Everybody writes bad code from time to time, and the more people you work with, the more likely you are to encounter cases. Linus is sure working with a lot of people.
Second, Linus has always been rude. This isn't because his beloved work of 30 years got sullied, it's because he has a temper and can get away with it. He had that temper back in 1991, and he's been at most slightly successful at tempering it since.
He's repeatedly making the choice to be that way. He could choose a different response, but he prefers replying this way, for whatever reasons.
You can argue if that is or isn't OK. But either way, it's a choice to be rude, not an innate emotional response because some dirty peon has sullied the œeuvre of the great artist.
I had "pleasure" of working with this type of people with inflated egos and too attached to their work, so I can certainly picture it, but not in a way you think.
Be humble.
People have become so fragile in the last decade or so. It's pathetic.
Would you speak to your teammates this way? Your child?
Note: treat Google engineers like literal toddlers.
Edit: Treating someone like a child is probably demeaning enough either way.
Google is functionally a third party vendor submitting their own solution for a technical problem the Chromium team wants fixed by changes to the kernel. That's the lens through which the response should be seen imo.
Google isn't a coworker, a teammate or a friend in this situation. They're a third party entirely.
Firstly: Torvalds does not owe Google anything beyond basic respect for sending him a patch; they don't pay his wages, nor does the LKML have Google as a business customer (Google does donate to the LF but only 3% of the LFs actual funding goes to kernel development and therefore Torvalds' wages). That basic respect is given - Torvalds does not insult or demean the submitters technical skills, merely the implementation they ended up going with.
Secondly: As far as I can tell, this proposal was brought forward straight up with an implementation, rather than consulting beforehand with the Mailing List on if this is a good idea and what such a contribution would look like in the context of the kernel (given that the implementation isn't a bugfix). Especially for more "mature"/"large" projects, dumping patch proposals that add maintenance overhead as a form of "discussion" as a submission tends to be seen as a hostile move. This is just general FOSS good behavior and from what I can tell this patch went through Googles internal discussion lists rather than the LKML. Google, perhaps to some suprise, does not own the LKML. The rudeness in that regard is mutual and was arguably started by the patch being submitted in that manner to begin with. (Something which is indicated further in the thread by the fact that the author attempted to start a new patch series for their proposal rather than discuss the feedback they got, showcasing they have little care for actually working with the LKML to get the patch merged).
Thirdly: I frankly... just don't think this feedback is that harsh in the light of that dynamic? Sure, some of it uses strong language but I've been on the receiving end of way worse from university teachers and the like. "This is a fundamentally bad implementation that doesn't follow LKML guidelines, but I like the idea" is what a lot of it comes down to. Calling something bad is not a mortal sin; if it sucks it just... sucks. The specifics of how you respond to that come ultimately down to a complex set of interpersonal relationships, but given how this was proposed, the tone of "yeah don't like this implementation because it does thinks I think make zero sense and you broke LKML rules, go back to the drawing board" seems adequate.
I do think that there might be a bit of cultural dissonance at play though. The way the US (and to a lesser extent the UK) tend to handle feedback is just... a lot more "couched" in niceties. That's just... not as much the case in other countries. I can only speak for Europe but if you get feedback here, you get the feedback very directly - this is what the other person thinks of it and if they actively dislike it, you will hear it. In the UK/US this tends to be a lot more muted from what I've seen - actively calling out bad things as bad is discouraged (a frequent example is when someone says "it's alright" and they actually meant "I really dislike this idea" - in Europe you'll often just hear the latter), so when someone actually uses direct language it's seen as too aggressive.
> Christ. That's literally the remap_file_pages() system call definition. No way in hell does "ON_BEHALF_OF_KERNEL" make any sense in this context.
This could be
> That's the remap_file_pages() system call definition. "ON_BEHALF_OF_KERNEL" does not make any sense in this context.
If someone I didn't know or was unfamiliar with gave me a review like this I'd frown, but since Torvalds is known for this then I'd shrug it off probably. It's a good, thorough review though.
They put forth a premise, "With Linux and git being probably the most widely adopted open source software projects ever"
Then an inference, "Linus's approach is historical proof"
And a conclusion, "is the best way [to review code]".
Assuming we can accept the premise as true, you're left to attack the inference (whether or not Linus's approach is historical proof). You may disagree with the conclusion, but calling out a logical argument as a logical fallacy, and using that to dismiss the conclusion, is itself, a logical fallacy called a "fallacy fallacy"[2]
Similarly you can't just use a single anecdote to prove anything either.
I agree that the argument could be expressed better, but I personally understood the intention.
But sure. We can debate the premise.
If even only Linux and Git succeeded, out of hundreds of projects managed with hostility, that’s two projects; one guy. fWIW, out of hundreds of every other projects NOT managed with hostility (and similarly running on as many computers), most of those, that I’m aware of, aren’t flourishing like Linux is. Usually they suffer from attracting talent and people. Perhaps the hostility of Linus makes the projects more visible (bad news is good news in the PR world), and some people take it as a challenge to succeed there. Thus the project flourishes, vs. fail from lack of talent.
I’d also argue that I’ve (personally) let some truly shitty code into code bases at $dayjob over the years because there was simply no “nice way” to say it was so terrible. Trying to argue on technical merits would just go round-and-round until I gave up. I’m pretty sure I’ve used the same tactics a couple of times to merge some shitty code I don’t care about and actually want someone to rewrite if they ever need to change it.
The point is, had we have been hostile (and managed to not get fired), the code could be hundreds of times better. So, I’d be willing to bet, there isn’t a selection bias here and hostile behavior attracts talent (due to being highly visible; popcorn factor), forcing people to actually rethink really shitty code, and letting people be more candid about how they really feel.
“Jesus Christ, if you could just explain why then we could have a fucking discussion, yeah?”
Intuitively you already understand the point I am making :)
It conveys the same message without needless verbosity.
The second is how I'd consciously phrase it, because I myself prefer someone not tell me the first one.
Maybe Linus doesn't do second takes (and that's not a secret).
I've worked in environments where it was considered a little mean even to use the "request changes" button in GitHub, but it worked because people (1) got the hint when you left comments suggesting changes and (2) would be careful to address all of your suggestions before requesting review again. But I've also worked in an environment where people would try to sneak changes past you and then argue with you when they were caught. I don't think I've ever sworn in a code review comment but these days I can understand where Linus Torvalds is coming from. Things would be much nicer if people got hints and were careful, but we don't live in that world.
My friend tried to use a credit card in Japan, and instead of saying "no" the shopkeeper just bowed and said "excuse me." Meanwhile yesterday in San Francisco I saw a customer arguing with the cashier for five minutes at McDonalds about whether or not they could use a coupon twice.
Anyway there is no possible argument that the meanness of those replies is productive; it would clearly be more useful and direct to omit it and speak clearly about the problems instead of implying the person is an idiot for not understanding their mistakes.
Moreover, why are you "surprised"? Do you live in a some toxic alternate reality where people talk like that regularly? For most people these messages are the only time they'll ever see someone being that mean in a professional context in their lives.
You'd have every first year college student trying to get their name on the mailing list with some crappy commit that adds no value and only takes time away from meaningful work.
Then you've got to hand hold and coddle that person, because you have to be nice to everyone. Yeah that's not gonna scale very well.
I can see going easy on independent developers. But this is Google. One of the biggest licensors of Linux and a closed source competitor. Here they are adding a feature to a repo they need to use, and they're doing it half-assed.
This is a feedback loop. Don't submit shit code to the most important repo in the world. Ain't nobody got time for that. If you do it as a fortune 500 company we will publically shame you.
Some of y'all have never been earnestly told to get your shit together and it shows.
This constitutes a personal insult (calling him immature for his age), so you aren't walking your talk. In this comment you're going much further than Linus's review being discussed, which doesn't contain any personal insults.
Maybe you think you're giving zelon88 a taste of his medicine like this, but if truly believe that people are obliged to communicate in a corporate-friendly manner you wouldn't be dishing out personal abuse like this to make a point. Overt tit-for-tat abusive communication doesn't fly in a modern psuedo-friendly corporate environment; you have to be more clever about it. In other words, you should lead by example.
> Clearly nobody has ever done it to you!
You really think nobody has ever been curt or even rude with zelon88? That beggars belief. Of course he has been on the receiving end of it; everybody has. Don't waste your time with the "you don't know what it's like" argument, nobody is going to believe that. You're wasting your time and undermining yourself with this tit-for-tat approach.
well... no...
Outside that sort of situation, of course, do what you like - just as I did.
I cannot fathom the internal mindset of a person who says "I'm being a dick, which I know is cruel and bad, but it's okay because of <various political and social factors>". The "knowing it's wrong part" wins! That what it means for something to be wrong! It's still wrong if you rationalize it!
And that's before the fact that it's also worse at its actual goals, like preventing bad code and maintaining community and giving valuable feedback.
Why do there need to be consequences? Can't the maintainers simply ignore shitty code?
> You'd have every first year college student trying to get their name on the mailing list with some crappy commit that adds no value and only takes time away from meaningful work.
Trolls could already do this if they want. In fact, getting a rise out of Linus would be a troll's dream. Instead, the team could (should) simply ignore shitty code. Am I missing something?
Giving honest feedback is infinitely more useful to any engineer who's skin is thick enough to receive it.
No, sorry. Businesses bend over backwards to coddle people now. Things were much more straightforward a couple decades ago. You could tell people straight up they were doing a shit job. Now you need to dance around it to avoid upsetting people who are wasting people's time.
Honestly, code review like that would make me absolutely want to work elsewhere. It is absolutely a "good" way to make plenty of people want to have nothing to do with the project, or the reviewer personally.
Even if someone writes code that is bad, that can still be treated as a learning opportunity, with examples of what might be better, or what to avoid. Neither of those call for exclamations, or adopting a tone that makes it seem like the person has an attitude. If being neutral/professional is somehow difficult, then being terse is also an option that's even easier.
This is particularly funny because I remember that several years ago, he publicly apologized for being abrasive and promised to get better... but old habits die hard.
He has every right to run the project as he sees fit, but then, people here have every right to be mildly cranky about that too.
Like I've read some of Linus' older swear-laden rants back in the day; those could get unreasonably angry at people for what amounted to things like inconsistent commenting and basically had shit in them that amounted to "the author shouldn't ask me for anything ever again". That's not present here.
This is a direct code review that calls out (from what I understand) a number of bad design problems (hence: totally illogical and complete nonsense) and the fact that the author doesn't seem to have followed the kernel commit message rules. (For which, yes, the moniker "worthless" is appropriate for those messages - the kernel has pretty strict commit message rules.)
It's strong language but strong language of this stripe is only bad if it's excessive or attacks the wrong thing. I don't think it's excessive in this case; the patch seems like it's a poorly thought through solution and is a technical mess for a project that has strict rules about user space APIs (aka if a poor solution ends up being used, it'll be maintained for ages).
In general the response from Linus reads more like that of a certain type of uni teacher; "this is bad, go back to the drawing board, here's what you did wrong".
Finally, also keep in mind that this patch was submitted by a Google employee (they're representing their employer) and that as far as I understand it, by the time you get to contribute to the mailing list that has Linus doing code review, you're at least expected to be familiar with how the Kernel works - he's not in a position where he can scare the newbies[0]. I'd not put up with it if it were a colleague but that's also really not the dynamic here. Google is more akin to a third party vendor than anything else.
[0]: Some of the lower tree maintainers are though.
> > My complaints are not some kind of small "fix this up". These are fundamental issues.
Maybe he wasn’t direct enough!
These remarks are directed at specific problems wit the code. Linus did not call the author worthless, nonsensical or illogical. In engineering, conflating criticism of your work with personal criticism of yourself is a sin.
Which, you know, might be justified every now and then, but it's not justified in this case.
Sorry my message must've hit a traumatic chord on your brain, but I didn't say I was offended, just that it was aggressive.
Like watching wwa show, it's aggressive, but entertaining.
Please try to not project your thinking too much next time you read someone else's comment next time.