Oh Jesus. At least write some comments or something. The silent merge is always terrible because it slips in and introduces bugs that sometimes don’t show up for months.
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
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.
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
Oh Jesus. At least write some comments or something. The silent merge is always terrible because it slips in and introduces bugs that sometimes don’t show up for months.
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
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.
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