4 ms·

It’s a bad review. The snarky prose is annoying. He could be much more concise as well. “Won’t merge. Commit messages need far more detail. Don’t do bitmask tra

It’s a bad review. The snarky prose is annoying. He could be much more concise as well. “Won’t merge. Commit messages need far more detail. Don’t do bitmask translations. All of these operations should use the same mask instead of creating redundant ones. Rethink the logic here, not sure if it makes sense. This needs a rewrite, not small tweaks” …was really all it needed. The wall of text is more offensive than the insulting tone.

cmrdporcupine 3y ago
Ordinarily I'd agree, e.g. within a company or team, I would not accept this kind of tone, it's not how peers communicate 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.
- foooorsyth 3y ago
  >He has limited time in the day 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.
  - cmrdporcupine 3y ago
    Yeah I mean that's maybe fair, and I thought Torvalds had been making attempts to avoid this kind of tone. I still think even if he had been concise and terse he would have been perceived as slightly-rude though.
    - hedora 3y ago
      The proposed summary of the review is too terse and doesn't make any sense. I don't see any redundant prose in the original one. His concerns are clear, and he gives actionable suggestions for improvement. The tone + his opinion of the patch set is also clear. 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.
      - w0z_ 3y ago
        Your reply is pretty much spot on.
    - dmvdoug 3y ago
      My impression was he was avoiding outright abusing people. Abusing their ideas as stupid was always going to be on the table for him. (Not justifying it but I think people are fooling themselves if they’re imagining a purely softer, gentler version of him all around.)
  - djbusby 3y ago
    Mark Twain said "I didn't have time to write a short letter, so I wrote a long one". Seems like what's happening here.
    - fanf2 3y ago
      Pascal, not Twain https://quoteinvestigator.com/2012/04/28/shorter-letter/ https://quoteinvestigator.com/2012/04/28/shorter-letter/
      - djbusby 3y ago
        TIL, thanks! But also,Mark is credited, and where I heard it first.
        fanf2 3y ago
        Quote Investigator says: > 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.
- heresie-dabord 3y ago
  > the company doing the proposal here is Google. They [...] should have come in with a more-better-finished product. 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.
  - taeric 3y ago
    The part that I think many people have a hard time internalizing, is that this is almost certainly following Google internal "best practices" for how the code is organized and delivered. They famously have "code reviewers" that are there to make sure the code reads well. 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.
- eviks 3y ago
  The review above takes less time to write, how does your argument work again? Also, tone isn't about mentoring
deleted 3y ago
[deleted]
deleted 3y ago
[deleted]
dbsmith83 3y ago
Your review doesn't take the time to explain why something is a bad idea though. I didn't think his tone was that bad. I think it depends on how you imagine the conversation in real life. I would prefer to have my review be blunt but instructive
- eviks 3y ago
  Where is "why" in this repetitive review? Also, what is the values of bluntly repeating in a more aggressive style vs bluntly saying the same thing once in a more professional manner while also explaining the reason (lack of details) > First off, the simple stuff: the commit messages are worthless. Having check seal for mmap(2) as the commit message is not even remotely acceptable, to pick one random example from the series (7/8).
  - dbsmith83 3y ago
    Try reading the rest of the review and you will see multiple explanations for why things are not a good idea. I don't even think your cherry picked example is that repetitive. It sounds like a normal conversation to me.
    - eviks 3y ago
      I've read the rest of the review, but "cherry picking" another example wouldn't help when you ignore the issues of the first one and have to add qualifiers (yeah, it's not "that repetitive", it's just repetitive)
josefx 3y ago
> Rethink the logic here, not sure if it makes sense. 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.

cmrdporcupine 3y ago

Ordinarily I'd agree, e.g. within a company or team, I would not accept this kind of tone, it's not how peers communicate 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.

foooorsyth 3y ago
>He has limited time in the day 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.
- cmrdporcupine 3y ago
  Yeah I mean that's maybe fair, and I thought Torvalds had been making attempts to avoid this kind of tone. I still think even if he had been concise and terse he would have been perceived as slightly-rude though.
  - hedora 3y ago
    The proposed summary of the review is too terse and doesn't make any sense. I don't see any redundant prose in the original one. His concerns are clear, and he gives actionable suggestions for improvement. The tone + his opinion of the patch set is also clear. 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.
    - w0z_ 3y ago
      Your reply is pretty much spot on.
  - dmvdoug 3y ago
    My impression was he was avoiding outright abusing people. Abusing their ideas as stupid was always going to be on the table for him. (Not justifying it but I think people are fooling themselves if they’re imagining a purely softer, gentler version of him all around.)
- djbusby 3y ago
  Mark Twain said "I didn't have time to write a short letter, so I wrote a long one". Seems like what's happening here.
  - fanf2 3y ago
    Pascal, not Twain https://quoteinvestigator.com/2012/04/28/shorter-letter/ https://quoteinvestigator.com/2012/04/28/shorter-letter/
    - djbusby 3y ago
      TIL, thanks! But also,Mark is credited, and where I heard it first.
      - fanf2 3y ago
        Quote Investigator says: > 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.
heresie-dabord 3y ago
> the company doing the proposal here is Google. They [...] should have come in with a more-better-finished product. 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.
- taeric 3y ago
  The part that I think many people have a hard time internalizing, is that this is almost certainly following Google internal "best practices" for how the code is organized and delivered. They famously have "code reviewers" that are there to make sure the code reads well. 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.
eviks 3y ago
The review above takes less time to write, how does your argument work again? Also, tone isn't about mentoring

deleted 3y ago

[deleted]

deleted 3y ago

[deleted]

dbsmith83 3y ago

Your review doesn't take the time to explain why something is a bad idea though. I didn't think his tone was that bad. I think it depends on how you imagine the conversation in real life. I would prefer to have my review be blunt but instructive

eviks 3y ago
Where is "why" in this repetitive review? Also, what is the values of bluntly repeating in a more aggressive style vs bluntly saying the same thing once in a more professional manner while also explaining the reason (lack of details) > First off, the simple stuff: the commit messages are worthless. Having check seal for mmap(2) as the commit message is not even remotely acceptable, to pick one random example from the series (7/8).
- dbsmith83 3y ago
  Try reading the rest of the review and you will see multiple explanations for why things are not a good idea. I don't even think your cherry picked example is that repetitive. It sounds like a normal conversation to me.
  - eviks 3y ago
    I've read the rest of the review, but "cherry picking" another example wouldn't help when you ignore the issues of the first one and have to add qualifiers (yeah, it's not "that repetitive", it's just repetitive)

josefx 3y ago

> Rethink the logic here, not sure if it makes sense. 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.