• prole [any, any]@hexbear.net
    link
    fedilink
    English
    arrow-up
    5
    ·
    13 hours ago

    I basically review every PR now because of this kind of shit. So many people who will approve code that doesn’t even work or has obvious issues. Honestly they are blatant too, I’ve been in meetings where someone asks for a review and another person approved it right then, faster than anyone could possibly look at it much less actually run it. I don’t even think our bosses care. I only care because it makes the team’s life harder long-term

    • invalidusernamelol [he/him]@hexbear.net
      link
      fedilink
      English
      arrow-up
      3
      ·
      12 hours ago

      PRs should always be atomically testable and in the absence of CI/CD workflows, the submitter should include test results that a reviewer should verify and new tests for their added functionality.

      I love using stuff like pytest for testing, but too often it gets out of sync too and it makes reviewers complacent. Including new tests makes it easier to review as you’ve defined behavior not just through implementation, but also the testing code.

      • prole [any, any]@hexbear.net
        link
        fedilink
        English
        arrow-up
        3
        ·
        12 hours ago

        Yeah, I wish. The app I work on is ancient and it’s just a clone of an even older app. And the whole time we’re fixing the old one we’re making a new version. We have a lot of testing established, but a lot of our devs won’t even add tests without a change request