• sugar_in_your_tea
    link
    fedilink
    English
    arrow-up
    8
    arrow-down
    2
    ·
    edit-2
    10 hours ago

    True, but that only applies if it’s misleading. For example:

    // pythagoran theorem 
    distance = abs(p2.x - p1.x) + abs(p2.y - p1.y); 
    

    Fixing that makes sense because it’s wrong and misleading (it’s actually Manhattan distance), and a quick glace is insufficient to tell the difference.

    But fixing a typo or something that wouldn’t be confusing is just noise and should only be fixed with other changes. For example, I intentionally misspelled Pythagorean in my comment above, fixing that to be the right spelling would be a useless change, even if the distance formula used the hypotenuse. It wouldn’t be an unreasonable policy to reject PRs that only fix spelling or similar to reduce noise for the maintainers.

    • surewhynotlem@lemmy.world
      link
      fedilink
      English
      arrow-up
      3
      ·
      8 hours ago

      Yep, I understand but disagree. Maybe it comes from working with so many ESL coders, but I’ll happily accept typo corrections because it’s not always obvious what words should be if you’re not steeped in the culture.

      • sugar_in_your_tea
        link
        fedilink
        English
        arrow-up
        2
        arrow-down
        1
        ·
        edit-2
        8 hours ago

        It really depends on the project.

        If you’re a larger project, you can see a ton of these from people hoping to land a commit to put “contributor to X” on a resume somewhere. Those add up and are really distracting and possibly automated. They waste everyone’s time, especially if they spawn a bunch of conversion like this did.

        If you’re a smaller project, it doesn’t matter as much. I work with ESL coders too, so I get it (1/4 of my office is ESL immigrants, and ~2/3 of the broader team is ESL). I fix comments all the time, I just include them with other changes.

        So it depends. But in general, a high profile project should reject this noise to discourage this behavior.

        • pogmommy@lemmy.blahaj.zone
          link
          fedilink
          English
          arrow-up
          3
          ·
          7 hours ago

          In theory that’s fair reasoning. Unfortunately the dev made it clear that his reasoning was based on politics

          • sugar_in_your_tea
            link
            fedilink
            English
            arrow-up
            2
            arrow-down
            1
            ·
            7 hours ago

            Did he? I only saw him point to the rule against politics.

            He should have said it’s because the PR isn’t worth the time, but it also seems motivated by something that’s against the rules (i.e. why make a PR that only fixes gender in one comment? There was a later PR that was accepted that fixed it in several places).

            So without more evidence, I cannot say what the dev’s motivations for rejecting the PR were, aside from the apparent rule breakage mentioned. They didn’t say they disagreed with the change (i.e. that the change was wrong), just the proposal of the change (i.e. seems more motivated by virtue signaling instead of improving the dev experience). And you can look at the comments and see justification for that position, since it quickly devolved into actual politics with people accusing the dev of being a Nazi.

            Maybe if you showed a pattern across more than just this incident (i.e. over months or years), but this sounds more like people being stubborn than tolerant.