• SkyNTP@lemmy.ml
    link
    fedilink
    arrow-up
    7
    ·
    8 months ago

    This wouldn’t pass PR review and automated tests, unless they were a senior dev and used elevated privileges to mess with things behind the scenes.

    • maynarkh@feddit.nl
      link
      fedilink
      arrow-up
      10
      arrow-down
      1
      ·
      8 months ago

      It’s bold to assume those exist. Maybe there’s a reason the coworker left

    • steal_your_face@lemmy.ml
      link
      fedilink
      English
      arrow-up
      8
      arrow-down
      1
      ·
      edit-2
      8 months ago

      Write a 5 line PR and receive 5 comments. Write a 500 line PR and receive no comments.

    • frezik@midwest.social
      link
      fedilink
      arrow-up
      3
      ·
      8 months ago

      rand() will be infrequent < 10 (at least ten in 2^15 times, if not exponentially more), so automated tests are likely to pass. If they don’t, they’re likely to pass on the second try, and then everyone shrugs and continues. If it’s buried in 500 other lines, then it’s likely the code reviewer will give it all a quick scan and say “it’s fine”. It’s the three line diffs that get lots of scrutiny.

      In other words, you seem to have a lot more faith in the process than I do.

      • sunbeam60@lemmy.one
        link
        fedilink
        arrow-up
        0
        ·
        8 months ago

        Yeah but even a single automated test would catch it and reject the PR. You just need a single test.

        • frezik@midwest.social
          link
          fedilink
          arrow-up
          1
          ·
          8 months ago

          No, you can’t assume that. The probability of hitting the condition each time is low. If there aren’t very many calls that hit this, it could easily slip through. Especially on 64-bit int platforms.