Don't (also) fix broken windows

You’re supposed to fix That Bug. That’s it. But the code is so ugly. Oh my gosh no wonder they wrote a bug here. I should tidy up. I’ll fix the “broken windows” while I’m here.

Refactor this name to be more descriptive. DRY up these three methods into one. Leave a comment. There, bug fixed and I left the code cleaner than I found it. I’m such a good boy.

Stop. Take a breath. Now revert everything but the bug fix.

When you’re making a change, just make that change. Don’t get distracted. Don’t clean up other code. Just make your change. If you want to fix it up. Clean up those broken windows. Then do it in a separate PR. It’s really easy. Just not this one.

Why?

  • Most importantly, the whole change will merge faster.

  • Separate PRs have will fewer bugs because complexity is non-linear. Two separate PRs are less complex than the same code in one combined PR. And complexity is where bugs make their home.

  • The reviewer will be able to more quickly ascertain which parts of the code pertain to the change, because all of them do, and put a clearer picture together of the change.

  • If there happens to be an issue with the unrelated fix, it could hold up the core bug fix.

  • As you have less code per change, code reviews will be higher quality.

  • The git history will be easier to read. Someone who does a git show on this commit to see what changes were made will be able to more quickly ascertain which parts of the code are more valuable.