Do you think the way he reviews PRs sets a bad example for engineers who've just graduated and look up to him? I mean, there are different ways to reject a PR, even if the code is garbage.
Or maybe he was upset because he expected a lot more from someone who works at Google and has been working on RISC-V since 2019?
I don't know Linus, so I'm not sure what to make of this.
Yeah, it's probably not a great example, but you have to put it into context. Also, whatever problems there may be in Linus's communication style, are just about general professional communication and not specific to code review.
I read the message again to understand the actual issues being discussed. There are two: (1) the patch came too late in the merge window, and (2) the patch adds an unnecessary and obfuscating helper function. I don't have an opinion on (1), but I think Linus is completely right about (2). Calling it garbage is pretty harsh but not really wrong, so I wouldn't even say it's particularly over-the-top.
Or maybe he was upset because he expected a lot more from someone who works at Google and has been working on RISC-V since 2019?
I don't know Linus, but I doubt "works at Google" carries much weight, and frankly that's an odd thing to focus on. If I had to guess, he probably read the patch, decided it was garbage, and wrote that in a message, not caring about where it came from. That's what I would have done (except I probably wouldn't have explicitly called it garbage, because I'm not Linus and that's not my style).
Comments
Do you think the way he reviews PRs sets a bad example for engineers who've just graduated and look up to him? I mean, there are different ways to reject a PR, even if the code is garbage.
Or maybe he was upset because he expected a lot more from someone who works at Google and has been working on RISC-V since 2019?
I don't know Linus, so I'm not sure what to make of this.
Yeah, it's probably not a great example, but you have to put it into context. Also, whatever problems there may be in Linus's communication style, are just about general professional communication and not specific to code review.
I read the message again to understand the actual issues being discussed. There are two: (1) the patch came too late in the merge window, and (2) the patch adds an unnecessary and obfuscating helper function. I don't have an opinion on (1), but I think Linus is completely right about (2). Calling it garbage is pretty harsh but not really wrong, so I wouldn't even say it's particularly over-the-top.
I don't know Linus, but I doubt "works at Google" carries much weight, and frankly that's an odd thing to focus on. If I had to guess, he probably read the patch, decided it was garbage, and wrote that in a message, not caring about where it came from. That's what I would have done (except I probably wouldn't have explicitly called it garbage, because I'm not Linus and that's not my style).