Linus Torvalds and the Supposedly "Garbage Code"
giodicanio.com
giodicanio.com
You all need to grow thicker skin. If we're really engineers, we can't dance around criticism. Linus' comment was about the code; he never insulted the developer. If you attach your self worth to the code you write that's your problem.
What would you have him do? Sugarcoat it? "Uh actually sweetie please rework this function it's not quite there".
It's a PR; honest, unbiased feedback on the code (and optionally how to improve it) is the only thing that matters. If the code is garbage then the reviewer needs to say so.
To be honest, some of those reactions did feel a bit out of proportion; I am not entirely sure if there could be other reasons for blowing Kent's attitude out of proportion: (Like, does someone stand to benefit that a COW-filesystem without too many sharp edges isn't shipped?).
Would you like to work at a place where these words are used to give feedback? I sure wouldn’t. But maybe I’m a snowflake.
So the commentary was on more than just the code it was on the entire situation. It was concluded by saying they could do this but not in generic headers and they could try the PR again early in the next release.
That language was not meant to be inflammatory but stern and the door was left wide open for future submissions.
Would I mind this in my workplace? Probably not. Particularly if my boss was a well known programmer with decades of experience and inside knowledge to share with me.
I get the sense that he’s more upset by the timing, and whilst unimpressed with the code, he’s reacting because of the timing. Hard to tell though because of the way that email is written.
Context matters here.
That way allows for tyrants.
I’m not saying it’s OK for everyone. If folks are unable to extract the same value that’s fine, find advice from someone else.
> No. The quality of this code is dreadful and it came in too late. I asked for early pull requests because I'm traveling, and if you can't follow that rule, at least make the pull requests good.
Slight tweak.
> This adds various poor quality code that isn't RISC-V specific to generic header files.
garbage -> code
> I am extremely upset that you have sent me such badly written code so late in a merge window.
you know, he's not so upset with the code, but he's upset with the timing. What's the bet he might have been a bit nicer if it had been sent in well before the merge window.
> Take for instance this make_u32_from_two_u16() helper function. This function makes code incomprehensible, and actively WORSE than not using it in the first place.
"That thing makes the world actively a worse place to live." but does it? Lots of things make the world a worse place to live in. Disease, crime, DOGE... but does a helper function actively degrade the ability to live in the world?
> If you write the code out as "(a << 16) + b", you know what it does and which is the high word. Maybe you need to add a cast to make sure that 'b' doesn't have high bits that pollutes the end result, so maybe it's not going to be exactly _pretty_, but it's not going to be wrong and incomprehensible either.
See, was that hard? I didn't change any of this.
> In contrast, if you write make_u32_from_two_u16(a,b) you don't know what the word order is. IOW, you just made things WORSE, and you added that helper function to a generic non-RISC-V file. If others people use this, then it will make other code worse too.
I hardly changed this, but I didn't sugar coat it.
> So no. Please don't include these sort of functions in the code. It does not go into generic header files, and it damn well does not happen late in the merge window.
Note I kept in the "damn well".
> You're on notice: no more late pull requests, and no more unrelated code outside the RISC-V tree.
This is fine. Torvalds is upset, for good reason.
> Now, I would hope there's no similar code inside the RISC-V parts, but that's your choice. But things in generic headers do not get polluted by this sort of stuff. And sending a big pull request the day before the merge window closes in the hope that I'm too busy to care is not a winning strategy.
Smallest tweak.
> So you get to try again in 6.18. EARLY in the that merge window. And without the garbage.
No change, even got a chance to use the word "garbage".
Perhaps I should put this in a way that you might enjoy though: Torvalds is a dick in his communication and a stunted human being. His inability to communicate to others makes them feel bad despite the fact they evidently put in a lot of time and effort to make things better and when they make him angry he can be a petty little shithead who has no control over his emotions.
See, you're talking about the person, whereas Linus was talking about the code and the behavior of submitting such code late. Linus' thing is much nicer
> That thing makes the world actively a worse place to live
Is really necessary to get the point across.
Software engineering is a collaborative process, not an adversarial one.
The collaborative process itself is adversarial. Capitulating to others when their contributions go against one's goal compromises that goal. Sometimes compromise to achieve a lesser goal is better than failing to achieving the full goal. But, when stakes are high (and Linux stakes are enormous) compromised goals are less appropriate. Linux and Linus are in a position not to have to compromise the goal.
Google the name of the receiving engineer. Despite the fact that he was involved in the creation of RISC-V, nearly all of the top results have the name “garbage” in them. I’d be mortified if that happened to my professional reputation. It would mentally destroy me. I don’t have the thick skin to deal with the ridicule of thousands.
For that reason alone I would never ever consider participating in Linux kernel development, and I’d advise others the same.
It is easy to critique others works without turning it into a spectator sport. Other people manage to do it all the time.
As for the “sweetie” part of your comment: there’s a world out there is that more than just black and white. You should try it sometime.
That’s not bad PR for a bad PR
In practice, how we communicate with each other matters, even in fields like engineering. That doesn't mean perfect objectivity all the time (that's not how anyone speaks to each other), but it does mean taking the time to understand how our words are going to be received by the person we're talking to.
Think of it this way: if I wanted to ask you to go to the shop, but instead of saying "shop", I kept on using a gibberish word instead, we'd say that was bad communication. I know you won't understand the gibberish word, so you won't understand what I'm trying to say to you.
The same is true of tone. Tone is part of communication, so when I use a tone of voice that is too aggressive, or too polite and careful, then you can't understand what I'm actually trying to say. Finding the right tone is necessary for good communication. What that tone is will depend on who I'm talking to - with my friends I'll use one tone and with strangers another; with British people I'll phrase things one way, with Germans in a different way.
Fwiw, I understand the communication style on the a Linux mailing list to generally be fairly combative, so if that's the culture Linus has built up, then fair enough if he chooses to communicate that way. Although the danger there of course is that if you only communicate like that and someone new appears who is not used to that culture, then there are going to be translation difficulties that may put newcomers off.
It is not an attack on the coder, just the work produced.
If we are to infer that any criticisms of our work are criticisms of ourselves then we are in deep trouble as a society
I feel like there's a middle ground here, where I understand that failures in my work don't necessarily define me, but that I am still responsible for them.
More specifically, it's nice to be polite, which Linus usually is.
Having a strong opinion about code is part of his job. Wording that opinion great every time and never showing any frustration, while working mostly with public, text based, permanently visible communication, is a pretty impossible standard.
It's better to be polite of course, but I wouldn't call someone an asshole or a bully because they sometimes fail at that.
That's... That's a hard one to even entertain as being correct..
Haha. Um what?
You are literally describing parenting, which has an extremely valuable role in society.
On a broader note, I recently had someone quote that classic adage "great minds discuss ideas, average minds discuss events, simple minds discuss people" at me and I just had to push back. Conversing about the behavior of others is how, by design, humans negotiate cultural values, morals and etiquette. That's not something to be sneered at.
Are you using the wrong word?
Parenting is what parents do towards children that are _their own_. Not random people on the Internet...
When you write explicit code you know what types the a and be are in the context.
You don't write generic code but explicit specific code for that context. In some contexts you will need a cast and in some others you won't (hence "maybe").
The rest of the article reads like a parody. Instead of a clear, explicit one-liner the author arrives at a multiline function which still doesn't solve the main issue Linus pointed out (that you don't know which argument is high and which is low when you read the code and encounter that function).
It's defined in the function parameters. Surely your IDE would let you know the correct order? Otherwise, you could make that comment about every single function you encounter.
The alternative is using build in common operators in the language.
>>Otherwise, you could make that comment about every single function you encounter.
It's a trade-off. The more logic there is to pack the more it's worth it. Here we have a one liner with two built in operators.
```rust
warning: manual implementation of `.is_multiple_of()`
| if number % i == 0 {
| ^^^^^^^^^^^^^^^ help: replace with: `number.is_multiple_of(i)`> No. This is garbage and it came in too late. ... This adds various garbage that isn't RISC-V specific to generic header files.