11 ms·
Linus Torvalds and the Supposedly "Garbage Code"
- Danjoe4 1y ago> I think Linus should be more respectful of other people 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.
- deredede 1y agoYou can give honest, unbiased feedback without insulting either people or their work. Software engineering is a collaborative process, not an adversarial one.
- frumiousirc 1y ago> 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.
- detaro 1y agoIn what way would rejecting the code with different wording be "capitulating" exactly?
- timmytokyo 1y agoIf you've worked in SWE for long enough, you'll run into this kind of socially maladjusted petty tyrant many times in your career. It's fascinating to see so many of these tyrants exposing themselves in this thread, though.
- frumiousirc 1y agoIn what way was I addressing Torvalds' choice of "wording", exactly?
- detaro 1y agoThat's the part the comment you replied to had a problem with, so it seemed obvious that your disagreement was with that.
- rolandog 1y agoThe author does have a point; the whole BcacheFS drama was in part because of comments that criticized other filesystem's design (granted, the straw that broke the camel's back where comments alluding to a developer needing to get their head checked out, and some faux-pas like adding a feature when a change freeze was already in place).
- procaryote 1y agoI had the impression it was about repeatedly ignoring the development process, and assuming the experimental bcachefs module moving quickly was somehow more important than the rest of the kernel being stable
- rolandog 1y agoYes, agreed. Though that's what I meant by "in part": when you read the threads, they derail when Kent takes potshots at other filesystems ... whose developers were CC'd in the mailing list. 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?).
- foreslion 1y agoHere are some words Linus used to describe the submitted code: crazy, stupid, garbage, needs to get bent, incomprehensible. 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.
- themafia 1y agoThe context is, pull requests of this magnitude were expected before the merge window, and this was sent after and made these modifications to generic header files used in other sub-trees. This has also happened previously. 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.
- chris_wot 1y agoThe thing is, if he got this patch at the right time, what’s the bet he would have said this is bad code, gave the same feedback but toned it down? 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.
- almost_usual 1y agoIf it came from an engineer as respected as Linus, sure. Someone who was average at best with a big ego? No. Context matters here.
- chris_wot 1y agoSo if you are a respected peer, it’s fine to abuse others, because you’ll get away with it? That way allows for tyrants.
- chris_wot 1y agoYou know, you can actually express your feedback without derogatory remarks. You don't need to sugarcoat it. Let's try that message from Torvalds again, without the diatribe and invective: > 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.
- kabes 1y agoYeah, but not sure if > That thing makes the world actively a worse place to live Is really necessary to get the point across.
- tverbeure 1y agoIt’s a very public email list and whenever Linus has one of these comments, they get widely shared. 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.
- MrJohz 1y agoDoesn't that go both ways? If we're "really engineers", we don't need to use hyperbole to communicate simple ideas like "this isn't on time, nor is it up to the right quality standards, and I'm tired of having to say this so often". 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.
- readthenotes1 1y agoHe's describing the code, not the coder. 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
- MrJohz 1y agoHuh, I would have said almost the opposite to that last paragraph. Recognising that I am the person who produced some bit of work, and therefore I am responsible for its quality (and in this case timeliness) is the heart of self-reflection. Without that, how can I ever get better at what I do? If criticisms of my work are completely divorced from any criticism of myself, then I cannot grow, because I am never being criticised. 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.
- bluecalm 1y ago>>First, this explicit code is likely wrong, and in fact Linus adds that “maybe you need to add a cast”. 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).
- SquareWheel 1y ago> 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.
- bluecalm 1y agoYou will not always have an IDE showing function definitions when reading diffs, greps or PR requests. Even if you do you still need to spend time to click and see the function definition and then read it. Argument names are helpful but they are not documentation so you still need a bit of time to process it. 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.
- majewsky 1y agoIt's not a function, it's a macro. Just a text replacement rule without defined argument types.
- procaryote 1y agoJust genereally, going around reviewing other people's discussions isn't really a valuable role in society 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.
- jackblemming 1y agoI have never seen myself or anyone I work with ever call another developers code “garbage”. Asshole behavior is never OK. Doesn’t matter if you’re Linus, Steve Jobs, or the other endless list of bullies who happen to have some talent.
- pixelat3d 1y agoCan't say I agree. As far as things that could be said go, "garbage" is pretty tame
- procaryote 1y agoThat's amazing, for me it's been a pretty common occurence. I've written garbage code that has been called out; sometimes impolitely. I've called out garbage code; sometimes impolitely. 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.
- Nervhq 1y agoLol linus torvalds, Steve Jobs, are... Bullies who have "some talent" That's... That's a hard one to even entertain as being correct..
- mustache_kimono 1y ago> More specifically, it's nice to be polite, which Linus usually is. Haha. Um what?
- 1y ago
- Arech 1y agoYour don't need noexcept on it, complier sees it on its own without a potential noexcept overhead... Other than that - agree.
- setterle 1y ago[flagged]
- deleted 1y ago[deleted]
- amadeuspagel 1y agoI'm always weirded out by how much attention Torvald's rants get. It seems a bit like the joy people get from watching a talent show and listening to the host insult the singers.
- procaryote 1y agoBuilding a kernel is hard, having opinions about things you're not involved in is easy
- timmytokyo 1y agoI think it's because most software developers have experience working with assholes. Commenting on someone famous who's being an asshole is a natural way to vent.
- Tade0 1y agoI think this rebuttal misses the point, which is explained in the first two paragraphs: > No. This is garbage and it came in too late. ... This adds various garbage that isn't RISC-V specific to generic header files.
- BoredPositron 1y agoYou missed the first part of Linus comment or ignored it completely and wrote a "Garbage Article" about it.
- zparky 1y agosource message, since the lkml link wasnt loading for me: https://lore.kernel.org/lkml/CAHk-=wjLCqUUWd8DzG+xsOn-yVL0Q=O35U9D6j6=2DUWX52ghQ@mail.gmail.com/ https://lore.kernel.org/lkml/CAHk-=wjLCqUUWd8DzG+xsOn-yVL0Q=...
- dlahoda 1y agoLatest Rust stable version of official default linter says: ```rust warning: manual implementation of `.is_multiple_of()` | if number % i == 0 { | ^^^^^^^^^^^^^^^ help: replace with: `number.is_multiple_of(i)` ```