12 ms·

It would have been more appropriate to just give credit to this guy even without accepting his full patch. Clearly no one was fixing this security issue unless

by ohyes 3y ago

It would have been more appropriate to just give credit to this guy even without accepting his full patch. Clearly no one was fixing this security issue unless he found it and submitted a fix. It’s unethical to not give credit, in particular if Michael read his patch, changed some stylistic things, and submitted it himself. I’m surprised everyone thinks the maintainer is in the right here. If someone scooped up your work and took full credit for it, despite it being a collaboration (and even if miculas code was absolute garbage it was still a collaboration), that wouldn’t feel right, and you wouldn’t want to work with them again.

Ajedi32 3y ago
He was credited. The commit says "Reported-by: Ariel Miculas" which seems accurate to me.
- mfru 3y ago
  "Reported-by tag gives credit to people who find bugs and report them and it hopefully inspires them to help us again in the future." [1] Nothing about that says "I have debugged an issue and contributed actual code to fix it" in there. [1]: https://docs.kernel.org/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes https://docs.kernel.org/process/submitting-patches.html#usin...
  - jxramos 3y ago
    I agree, it obscures all the heavy lifting debugging and validating and certifying with strong confidence the fix works and continues to work, the guy even tested it on newer kernel releases. Someone who exudes that much perseverance should be welcomed with open arms. These are the kind of go getter get stuff done type of individuals every company longs for.
  - loeg 3y ago
    The thing non-kernel people (including this guy) don't seem to understand is that the debugging and investigation is much more valuable than the actual code fix. The author of the article and commenters here seem overfocused on the actual patch. But the valuable and hard part was identifying the issue. Reported-by gives credit for that. (Sure, in the same situation I would prefer to also be credited with authorship too. But I get it.)
    - FireBeyond 3y ago
      And when your fix was the actual fix, but the maintainer rewrote it stylistically? Why should the maintainer get that credit?
      - loeg 3y ago
        The author's proposed patch had issues raised in review that were never addressed. I disagree with the characterization that the differences between the two authors' patches were purely cosmetic.
    - CydeWeys 3y ago
      Reporting a bug is a lot less credit than debugging and fixing a bug. He did the latter, even if his exact fix didn't end up being the one merged into the repo. Generally most of the work occurs after a bug is reported, as it did here.
    - matrss 3y ago
      Reported-by should go to the person who reported the issue 6 years ago. Finding the issue (again), investigating, debugging, fixing and testing that fix sounds more like a hypothetical "Fixed-by", or co-authorship, depending on how similar the final patches really are. Authoring the patch is often trivial once the debugging is done. Describing the debugging as just reporting is underselling it.
      - jacquesm 3y ago
        That's fair, but before you know it you're going to have a whole taxonomy of different kinds of contributions and there simply isn't one for 'x did RCA' and the fact that the original patch wasn't properly signed off as mentioned in the exchange with the kernel maintainer may have been all that stood between that and another level of accreditation. Which by the way wasn't responded to by the OP as far as I can see, even though the maintainer clearly mentioned it and the LKML list spells out that that is a requirement for accreditation.
        3y ago
  - Ajedi32 3y ago
    There's no such thing as a "have-debugged-and-contributed-actual-code-to-fix-it:" tag. He didn't write the actual commit that fixed the issue, so he was credited in the commit message instead. That seems fine to me. The fact that they didn't include an entire paragraph detailing the exact nature of his contribution isn't a reason to blow this up into some big community drama with over 500 comments on Hacker News.
    - Snild 3y ago
      > There's no such thing as a "have-debugged-and-contributed-actual-code-to-fix-it:" tag. "Based-on-patch-by:" and/or "Root-caused-by:" comes pretty close, I think. It may seem like a small difference, but I understand the author's disappointment.
      - Sprocklem 3y ago
        Those might be better, but neither is a tag that is used in kernel commits. There is a list of specific tags that are used, and specific conditions on their use. "Reported-by:" was not a wholly arbitrary choice of words by the maintainer.
        Snild 3y ago
        AFAIK, there are no rules limiting what tags you may use -- you can make up your own if you like. And people have. Here's what the kernel git says right now: linux$ git log | sed -nE 's/^ *([^ ]+-by:).*$/\1/p' | tr '[:upper:]' '[:lower:]' | sort | uniq -c | egrep -v ' [0-9] ' 20 ack-by: 74 acked-and-tested-by: 195801 acked-by: 24 acked-for-backlight-by: 49 acked-for-mfd-by: 19 acked-in-principle-or-something-like-that-by: 21 acked-off-by: 10 analysed-by: 47 analyzed-by: 26 based-on-a-patch-by: 97 based-on-patch-by: 15 based-on-work-by: 128 bisected-by: 18 boot-tested-by: 37 build-tested-by: 147 co-authored-by: 4779 co-developed-by: 175 debugged-by: 60 diagnosed-by: 11 disliked-by: 11 eviewed-by: 51 fixed-by: 18 fix-suggested-by: 32 found-by: 48 generated-by: 19 improvements-by: 79 inspired-by: 18 located-by: 10 not-acked-by: 86 noticed-by: 30 original-by: 140 originally-by: 95 original-patch-by: 44 pointed-out-by: 14 proposed-by: 10 reivewed-by: 22 reported-and-acked-by: 12 reported-and-analyzed-by: 77 reported-and-bisected-by: 12 reported-and-debugged-by: 23 reported-and-suggested-by: 3017 reported-and-tested-by: 21 reported-bisected-and-tested-by: 58990 reported-by: 11 requested-and-tested-by: 363 requested-by: 10 review-by: 55 reviewd-by: 513 reviewed-and-tested-by: 304543 reviewed-by: 16 reviewed-off-by: 67 reviwed-by: 11 root-caused-by: 29 sigend-off-by: 10 signed-by: 157 signed-of-by: 2253448 signed-off-by: 59 singed-off-by: 54 spotted-by: 32 suggested-and-acked-by: 12 suggested-and-reviewed-by: 17 suggested-and-tested-by: 15568 suggested-by: 48 tested-and-acked-by: 22 tested-and-reported-by: 35 tested-and-reviewed-by: 62630 tested-by: 12 verified-by: My suggestions are both there (far less often than "Reported-by:", but that's not surprising). I think "Analyzed-by:", "Debugged-by", "Diagnosed-by", "Originally-by:", or "Original-patch-by:" would also have worked. If you don't filter out the tags with less than 10 uses, there are some fun ones. For example, "You're-my-ding-a-ling-by:" should be used more often. :)
- kevincox 3y ago
  I think it depends a lot on how much the final patch was based on the one he wrote. There are a few phases of solving an issue: Identified: They have found an issue and reported it. Debugged: They found the cause of the issue. Fixed: They wrote the fix. It seems that Ariel should be credited for somewhere between Debugged and Fixed. But is only explicitly given credit for the first "tier" of solving the problem. It would have been nice to explain a bit more the work that Ariel put in. But I agree that I wouldn't expect an inferior (by the maintainer's perspective) patch to be included just to give that credit. But a sentence explaining how Ariel contributed would likely be much appreciated.
  - leereeves 3y ago
    Unfortunately the author wasn't comfortable sharing his patch ("this mailing list is private so I cannot link to the original patch I sent"), but a comment now buried deep in the discussion [1] says: > The only difference between the patch that was accepted and the one that was proposed is where the fix is. In one case it's in an ifdef outside of an if. In the other it's in an inner if statement. That's it. This is a difference in style not a technical difference in the patch at all. It sounds like the author wrote the fix and the maintainer merely edited it to fit the project style. If that truly is all that was done, how hard would it have been to ask the author to quickly make that change? 1: https://news.ycombinator.com/item?id=37672558 https://news.ycombinator.com/item?id=37672558
    - bluGill 3y ago
      IF the patch was only edited for style or other minor issues, then the copyright remains with the original contributor. If the patch was completely rewritten from scratch and is very different then not. (linux does not require copyright attribution) If the poster wants to get a lawyer he can probably force rewriting git history to give him credit. Is it worth it - probably not.
      - jacquesm 3y ago
        But the OP has copyright. The LKML archive is there for everybody to see, they wrote a bit of code. That code was then modified and that modified version was entered into the repo under the maintainers name, as it usually goes. And that's because when you mail LKML you play by their rules, which have been made very explicit: https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html https://www.kernel.org/doc/html/v4.17/process/submitting-pat... Note the bit about the Co-developed-by tag: "A Co-Developed-by: states that the patch was also created by another developer along with the original author. This is useful at times when multiple people work on a single patch. Note, this person also needs to have a Signed-off-by: line in the patch as well." and "Note, you did not properly sign-off on this commit, so it couldn't be applied anyway :(" From the exchange between the OP and the kernel maintainer. Which may have been the whole issue.
- bitcharmer 3y ago
  "Reported-by" is absolutely not the appropriate level of recognition for Ariel's input. This would be suitable if he just raised the issue with the maintainer along with some context. Instead, he put a lot of work into figuring out the nature of the problem and implementing a fix. This is very unethical conduct on the maintainer's part.
  - jacquesm 3y ago
    > This is very unethical conduct on the maintainer's part. And this is why finding good maintainers for visible FOSS projects is hard. You know what's unethical conduct? Misrepresenting someone's words in a way that make them look bad. The maintainer did exactly what they usually do, I see absolutely nothing unexpected here, note that this was an unsolicited patch sent to a security mailing list. And this is typically how those are dealt with. To claim 'unethical conduct' is one small step removed from a formal complaint about the maintainer, and which is exactly the wrong message to send here. Witch hunts like this cause good people to leave FOSS projects. The maintainer could have given more credit but chose not to, that's the rules you play by when sending in unsolicited patches. If you must have credit then you should not contribute in this fashion.
    - leereeves 3y ago
      > The maintainer could have given more credit but chose not to, that's the rules you play by when sending in unsolicited patches Is something like "we may accept your work and choose not to give you credit" actually stated somewhere when you sign up for or submit to that security mailing list? Because that's certainly not the usual rule for unsolicited work in other contexts.
      - jacquesm 3y ago
        You can read the LKML mailing list page for submitting patches: https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html https://www.kernel.org/doc/html/v4.17/process/submitting-pat... Absolutely nowhere does it say that accepting your patch will lead to you automatically being given kernel contributor status, and to require that status on the basis of committing a single tiny patch that wasn't properly signed off and required work shows a different set of expectations than one that I would see as reasonable.
- kazinator 3y ago
  Reported-by doesn't specifically express the idea that Miculas investigated the problem through to the root cause.
- m000 3y ago
  Reported by would be appropriate credit if (a) the maintainer did not read Ariel's code or (b) Ariel's code was utter crap, so it was effectively discarded. "I like my code better" indicates that neither of this happened. So, Ariel deserves (at least partial) credit for fixing, since this was not a "clean room" fix.
  - dorfsmay 3y ago
    And if Ellerman did not want to spend time going back and forth, he could have accepted Ariel's patch then immediately modify in a new commit.
    - jacquesm 3y ago
      For that it would have to be properly signed off. This was explicitly noted in the exchange and not remedied by OP.
    - opello 3y ago
      Why introduce risk in a bisect? A Co-developed-by or another appropriate attribution tag should solve this nicely? Lots of options it seems: https://news.ycombinator.com/item?id=37685190 https://news.ycombinator.com/item?id=37685190
  - nolist_policy 3y ago
    "I like my code better" is not what the maintainer said[1]: > Hi Ariel, > > I've added Christophe to Cc who works on ppc32. > > I haven't actually reproduced the crash with gdbserver, but I have a > test case which shows the bug, so I've been able to confirm it and > test a fix. > > Thanks for your patch, but I wanted to fix it differently. Can you try > the patch below and make sure it fixes the bug for you? > > I've also attached the test case I've been using. > > Christophe are you able to test these on some 32-bit machines? I've > tested it in qemu and on one 32-bit machine I have here, but some more > real testing would be good. > > > If the patch works then I'll need to do manual back ports for several of > the stable kernels, and then once those are ready I will publish the > patch. > > cheers > > [...] [1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-June/244387.html https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-June/24...
alain94040 3y ago
Agreed. I see the job of an experienced maintainer as a facilitator who should bend over backward to help other people's contributions land cleanly. The help often involves feedback on style and architecture consistency, but a sign of a great maintainer is someone who credits others, and mostly stays behind the curtain making sure everything goes smoothly.
- CodeWriter23 3y ago
  And occasionally coming out from under the bridge to give guidance when a contributor gets too far into the weeds.
whats_a_quasar 3y ago
Agreed. The maintainer can insist on writing the code that makes it into the kernel for quality purposes - that makes sense. But here the article author is a co-author of that patch. In academic publishing, this is analogous to sending a pre-print to someone, and that person then publishing on that topic listing you in acknowledgments rather than as a co-author. I get that the point here isn't publishing, it's kernel development, but authorship is still quite important for a professional C.V.
anonymousiam 3y ago
The first few times I submitted suggested patches to the Linux kernel (before semi-automated submissions through the LKML were possible), I had a dialog with the maintainer (Ted Tso in this case). Once he was satisfied with the correctness of my work, he incorporated the patch and all was good. I never asked for, nor received any credit. I was solely interested in improving the kernel and not interested in getting my name in there. (My name ended up in the contributors list anyway for something else a few years later, but I never cared one way or the other.)
ljm 3y ago
I used to think it was fine to take someone else's contribution and then create a total rewrite of it in my own branch and merge that. Or to go into someone's PR and push a rewrite into a new commit on Github. At least in the latter case there'll still be a contributor credit (`Co-Authored-By`), but someone had to do the same with me for me to realise that it kinda sucked and really soured the experience of being a contributor. In my case, it was that someone 'forked' my project by dumping the code into their own repo without any of the commit history. As you say, even if the code was shite, the refactored fix that got merged in would never have existed without it. "I like my version better," feels pretty dismissive and regardless of any other fact I can see how anyone would feel a bit miffed about that after they put the work in. Edit: the reply linking to the actual response from the maintainer invalidates my last sentence.
- nolist_policy 3y ago
  But nobody said "Sorry, I like my version better. If you want to be a Linux kernel contributor, here’s an issue you could fix."! This[1] is what the maintainer (Michael) said. [1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-June/244387.html https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-June/24...
  - Aurornis 3y ago
    This is much more reasonable than the "paraphrased" reply in the blog post. I think it's somewhat dishonest that the blog author used the quotation feature in his blog post, but instead of including a quote he made up his own response as if the kernel maintainer said it. I barely noticed this parenthetical above the gigantic block quote: > He said (paraphrasing): Anyone reading the blog post too fast would assume the kernel maintainer actually said those dismissive words in the gigantic block quote.
    - orra 3y ago
      Yeah, the paraphrasing here is misleading. Ironically the topic is intellectual honesty.

by ohyes 3y ago

Ajedi32 3y ago

He was credited. The commit says "Reported-by: Ariel Miculas" which seems accurate to me.

mfru 3y ago
"Reported-by tag gives credit to people who find bugs and report them and it hopefully inspires them to help us again in the future." [1] Nothing about that says "I have debugged an issue and contributed actual code to fix it" in there. [1]: https://docs.kernel.org/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes https://docs.kernel.org/process/submitting-patches.html#usin...
- jxramos 3y ago
  I agree, it obscures all the heavy lifting debugging and validating and certifying with strong confidence the fix works and continues to work, the guy even tested it on newer kernel releases. Someone who exudes that much perseverance should be welcomed with open arms. These are the kind of go getter get stuff done type of individuals every company longs for.
- loeg 3y ago
  The thing non-kernel people (including this guy) don't seem to understand is that the debugging and investigation is much more valuable than the actual code fix. The author of the article and commenters here seem overfocused on the actual patch. But the valuable and hard part was identifying the issue. Reported-by gives credit for that. (Sure, in the same situation I would prefer to also be credited with authorship too. But I get it.)
  - FireBeyond 3y ago
    And when your fix was the actual fix, but the maintainer rewrote it stylistically? Why should the maintainer get that credit?
    - loeg 3y ago
      The author's proposed patch had issues raised in review that were never addressed. I disagree with the characterization that the differences between the two authors' patches were purely cosmetic.
  - CydeWeys 3y ago
    Reporting a bug is a lot less credit than debugging and fixing a bug. He did the latter, even if his exact fix didn't end up being the one merged into the repo. Generally most of the work occurs after a bug is reported, as it did here.
  - matrss 3y ago
    Reported-by should go to the person who reported the issue 6 years ago. Finding the issue (again), investigating, debugging, fixing and testing that fix sounds more like a hypothetical "Fixed-by", or co-authorship, depending on how similar the final patches really are. Authoring the patch is often trivial once the debugging is done. Describing the debugging as just reporting is underselling it.
    - jacquesm 3y ago
      That's fair, but before you know it you're going to have a whole taxonomy of different kinds of contributions and there simply isn't one for 'x did RCA' and the fact that the original patch wasn't properly signed off as mentioned in the exchange with the kernel maintainer may have been all that stood between that and another level of accreditation. Which by the way wasn't responded to by the OP as far as I can see, even though the maintainer clearly mentioned it and the LKML list spells out that that is a requirement for accreditation.
      - 3y ago
- Ajedi32 3y ago
  There's no such thing as a "have-debugged-and-contributed-actual-code-to-fix-it:" tag. He didn't write the actual commit that fixed the issue, so he was credited in the commit message instead. That seems fine to me. The fact that they didn't include an entire paragraph detailing the exact nature of his contribution isn't a reason to blow this up into some big community drama with over 500 comments on Hacker News.
  - Snild 3y ago
    > There's no such thing as a "have-debugged-and-contributed-actual-code-to-fix-it:" tag. "Based-on-patch-by:" and/or "Root-caused-by:" comes pretty close, I think. It may seem like a small difference, but I understand the author's disappointment.
    - Sprocklem 3y ago
      Those might be better, but neither is a tag that is used in kernel commits. There is a list of specific tags that are used, and specific conditions on their use. "Reported-by:" was not a wholly arbitrary choice of words by the maintainer.
      - Snild 3y ago
        AFAIK, there are no rules limiting what tags you may use -- you can make up your own if you like. And people have. Here's what the kernel git says right now: linux$ git log | sed -nE 's/^ *([^ ]+-by:).*$/\1/p' | tr '[:upper:]' '[:lower:]' | sort | uniq -c | egrep -v ' [0-9] ' 20 ack-by: 74 acked-and-tested-by: 195801 acked-by: 24 acked-for-backlight-by: 49 acked-for-mfd-by: 19 acked-in-principle-or-something-like-that-by: 21 acked-off-by: 10 analysed-by: 47 analyzed-by: 26 based-on-a-patch-by: 97 based-on-patch-by: 15 based-on-work-by: 128 bisected-by: 18 boot-tested-by: 37 build-tested-by: 147 co-authored-by: 4779 co-developed-by: 175 debugged-by: 60 diagnosed-by: 11 disliked-by: 11 eviewed-by: 51 fixed-by: 18 fix-suggested-by: 32 found-by: 48 generated-by: 19 improvements-by: 79 inspired-by: 18 located-by: 10 not-acked-by: 86 noticed-by: 30 original-by: 140 originally-by: 95 original-patch-by: 44 pointed-out-by: 14 proposed-by: 10 reivewed-by: 22 reported-and-acked-by: 12 reported-and-analyzed-by: 77 reported-and-bisected-by: 12 reported-and-debugged-by: 23 reported-and-suggested-by: 3017 reported-and-tested-by: 21 reported-bisected-and-tested-by: 58990 reported-by: 11 requested-and-tested-by: 363 requested-by: 10 review-by: 55 reviewd-by: 513 reviewed-and-tested-by: 304543 reviewed-by: 16 reviewed-off-by: 67 reviwed-by: 11 root-caused-by: 29 sigend-off-by: 10 signed-by: 157 signed-of-by: 2253448 signed-off-by: 59 singed-off-by: 54 spotted-by: 32 suggested-and-acked-by: 12 suggested-and-reviewed-by: 17 suggested-and-tested-by: 15568 suggested-by: 48 tested-and-acked-by: 22 tested-and-reported-by: 35 tested-and-reviewed-by: 62630 tested-by: 12 verified-by: My suggestions are both there (far less often than "Reported-by:", but that's not surprising). I think "Analyzed-by:", "Debugged-by", "Diagnosed-by", "Originally-by:", or "Original-patch-by:" would also have worked. If you don't filter out the tags with less than 10 uses, there are some fun ones. For example, "You're-my-ding-a-ling-by:" should be used more often. :)
kevincox 3y ago
I think it depends a lot on how much the final patch was based on the one he wrote. There are a few phases of solving an issue: Identified: They have found an issue and reported it. Debugged: They found the cause of the issue. Fixed: They wrote the fix. It seems that Ariel should be credited for somewhere between Debugged and Fixed. But is only explicitly given credit for the first "tier" of solving the problem. It would have been nice to explain a bit more the work that Ariel put in. But I agree that I wouldn't expect an inferior (by the maintainer's perspective) patch to be included just to give that credit. But a sentence explaining how Ariel contributed would likely be much appreciated.
- leereeves 3y ago
  Unfortunately the author wasn't comfortable sharing his patch ("this mailing list is private so I cannot link to the original patch I sent"), but a comment now buried deep in the discussion [1] says: > The only difference between the patch that was accepted and the one that was proposed is where the fix is. In one case it's in an ifdef outside of an if. In the other it's in an inner if statement. That's it. This is a difference in style not a technical difference in the patch at all. It sounds like the author wrote the fix and the maintainer merely edited it to fit the project style. If that truly is all that was done, how hard would it have been to ask the author to quickly make that change? 1: https://news.ycombinator.com/item?id=37672558 https://news.ycombinator.com/item?id=37672558
  - bluGill 3y ago
    IF the patch was only edited for style or other minor issues, then the copyright remains with the original contributor. If the patch was completely rewritten from scratch and is very different then not. (linux does not require copyright attribution) If the poster wants to get a lawyer he can probably force rewriting git history to give him credit. Is it worth it - probably not.
    - jacquesm 3y ago
      But the OP has copyright. The LKML archive is there for everybody to see, they wrote a bit of code. That code was then modified and that modified version was entered into the repo under the maintainers name, as it usually goes. And that's because when you mail LKML you play by their rules, which have been made very explicit: https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html https://www.kernel.org/doc/html/v4.17/process/submitting-pat... Note the bit about the Co-developed-by tag: "A Co-Developed-by: states that the patch was also created by another developer along with the original author. This is useful at times when multiple people work on a single patch. Note, this person also needs to have a Signed-off-by: line in the patch as well." and "Note, you did not properly sign-off on this commit, so it couldn't be applied anyway :(" From the exchange between the OP and the kernel maintainer. Which may have been the whole issue.
bitcharmer 3y ago
"Reported-by" is absolutely not the appropriate level of recognition for Ariel's input. This would be suitable if he just raised the issue with the maintainer along with some context. Instead, he put a lot of work into figuring out the nature of the problem and implementing a fix. This is very unethical conduct on the maintainer's part.
- jacquesm 3y ago
  > This is very unethical conduct on the maintainer's part. And this is why finding good maintainers for visible FOSS projects is hard. You know what's unethical conduct? Misrepresenting someone's words in a way that make them look bad. The maintainer did exactly what they usually do, I see absolutely nothing unexpected here, note that this was an unsolicited patch sent to a security mailing list. And this is typically how those are dealt with. To claim 'unethical conduct' is one small step removed from a formal complaint about the maintainer, and which is exactly the wrong message to send here. Witch hunts like this cause good people to leave FOSS projects. The maintainer could have given more credit but chose not to, that's the rules you play by when sending in unsolicited patches. If you must have credit then you should not contribute in this fashion.
  - leereeves 3y ago
    > The maintainer could have given more credit but chose not to, that's the rules you play by when sending in unsolicited patches Is something like "we may accept your work and choose not to give you credit" actually stated somewhere when you sign up for or submit to that security mailing list? Because that's certainly not the usual rule for unsolicited work in other contexts.
    - jacquesm 3y ago
      You can read the LKML mailing list page for submitting patches: https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html https://www.kernel.org/doc/html/v4.17/process/submitting-pat... Absolutely nowhere does it say that accepting your patch will lead to you automatically being given kernel contributor status, and to require that status on the basis of committing a single tiny patch that wasn't properly signed off and required work shows a different set of expectations than one that I would see as reasonable.
kazinator 3y ago
Reported-by doesn't specifically express the idea that Miculas investigated the problem through to the root cause.
m000 3y ago
Reported by would be appropriate credit if (a) the maintainer did not read Ariel's code or (b) Ariel's code was utter crap, so it was effectively discarded. "I like my code better" indicates that neither of this happened. So, Ariel deserves (at least partial) credit for fixing, since this was not a "clean room" fix.
- dorfsmay 3y ago
  And if Ellerman did not want to spend time going back and forth, he could have accepted Ariel's patch then immediately modify in a new commit.
  - jacquesm 3y ago
    For that it would have to be properly signed off. This was explicitly noted in the exchange and not remedied by OP.
  - opello 3y ago
    Why introduce risk in a bisect? A Co-developed-by or another appropriate attribution tag should solve this nicely? Lots of options it seems: https://news.ycombinator.com/item?id=37685190 https://news.ycombinator.com/item?id=37685190
- nolist_policy 3y ago
  "I like my code better" is not what the maintainer said[1]: > Hi Ariel, > > I've added Christophe to Cc who works on ppc32. > > I haven't actually reproduced the crash with gdbserver, but I have a > test case which shows the bug, so I've been able to confirm it and > test a fix. > > Thanks for your patch, but I wanted to fix it differently. Can you try > the patch below and make sure it fixes the bug for you? > > I've also attached the test case I've been using. > > Christophe are you able to test these on some 32-bit machines? I've > tested it in qemu and on one 32-bit machine I have here, but some more > real testing would be good. > > > If the patch works then I'll need to do manual back ports for several of > the stable kernels, and then once those are ready I will publish the > patch. > > cheers > > [...] [1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-June/244387.html https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-June/24...

alain94040 3y ago

Agreed. I see the job of an experienced maintainer as a facilitator who should bend over backward to help other people's contributions land cleanly. The help often involves feedback on style and architecture consistency, but a sign of a great maintainer is someone who credits others, and mostly stays behind the curtain making sure everything goes smoothly.

CodeWriter23 3y ago
And occasionally coming out from under the bridge to give guidance when a contributor gets too far into the weeds.

whats_a_quasar 3y ago

Agreed. The maintainer can insist on writing the code that makes it into the kernel for quality purposes - that makes sense. But here the article author is a co-author of that patch. In academic publishing, this is analogous to sending a pre-print to someone, and that person then publishing on that topic listing you in acknowledgments rather than as a co-author. I get that the point here isn't publishing, it's kernel development, but authorship is still quite important for a professional C.V.

anonymousiam 3y ago

The first few times I submitted suggested patches to the Linux kernel (before semi-automated submissions through the LKML were possible), I had a dialog with the maintainer (Ted Tso in this case). Once he was satisfied with the correctness of my work, he incorporated the patch and all was good. I never asked for, nor received any credit. I was solely interested in improving the kernel and not interested in getting my name in there. (My name ended up in the contributors list anyway for something else a few years later, but I never cared one way or the other.)

ljm 3y ago

I used to think it was fine to take someone else's contribution and then create a total rewrite of it in my own branch and merge that. Or to go into someone's PR and push a rewrite into a new commit on Github. At least in the latter case there'll still be a contributor credit (`Co-Authored-By`), but someone had to do the same with me for me to realise that it kinda sucked and really soured the experience of being a contributor. In my case, it was that someone 'forked' my project by dumping the code into their own repo without any of the commit history. As you say, even if the code was shite, the refactored fix that got merged in would never have existed without it. "I like my version better," feels pretty dismissive and regardless of any other fact I can see how anyone would feel a bit miffed about that after they put the work in. Edit: the reply linking to the actual response from the maintainer invalidates my last sentence.

nolist_policy 3y ago
But nobody said "Sorry, I like my version better. If you want to be a Linux kernel contributor, here’s an issue you could fix."! This[1] is what the maintainer (Michael) said. [1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-June/244387.html https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-June/24...
- Aurornis 3y ago
  This is much more reasonable than the "paraphrased" reply in the blog post. I think it's somewhat dishonest that the blog author used the quotation feature in his blog post, but instead of including a quote he made up his own response as if the kernel maintainer said it. I barely noticed this parenthetical above the gigantic block quote: > He said (paraphrasing): Anyone reading the blog post too fast would assume the kernel maintainer actually said those dismissive words in the gigantic block quote.
  - orra 3y ago
    Yeah, the paraphrasing here is misleading. Ironically the topic is intellectual honesty.