• FizzyOrange@programming.dev
    link
    fedilink
    arrow-up
    12
    arrow-down
    7
    ·
    5 days ago

    getting called out will make the person really review their next submission.

    Yeah or they’ll say “fuck this” and quit.

    The expectation that somebody always has to be nice to you while you fuckup, is not ideal.

    It’s hardly a fuck up. They named a function slightly poorly. As if Linus has never done that.

    • JamonBear@sh.itjust.works
      link
      fedilink
      arrow-up
      11
      ·
      5 days ago

      Cmon, this is not about naming, this is about non-generic code in generic header.

      IMO hiding such a little operation behind a macro/function just hurt readability. Furthermore, considering that this function is only used once in the provided patch and that word ordering on RISC-V is not about to change anytime soon, it is perfectly fine to inline the code.

      • FizzyOrange@programming.dev
        link
        fedilink
        arrow-up
        3
        arrow-down
        3
        ·
        4 days ago

        this is about non-generic code in generic header.

        (a << 16) | b is about the most generic code you can get. How is that remotely RISC-V specific?

    • nocteb@feddit.org
      link
      fedilink
      arrow-up
      10
      arrow-down
      3
      ·
      edit-2
      5 days ago

      They named a function slightly poorly. As if Linus has never done that.

      Not only that. They introduced a obscure function, which inner workings are not clear and that is only used by their new code into a global header which is used by many other code parts, which means other people could start using it. This could lead to bugs since the semantics are not clear from the function name or if they change the function in the future. Also they added their pull request much too late to be properly reviewed.