Yesterday I observed a debate about how to structure code pull requests:
- is it reasonable to keep functional and “cosmetic”/refactoring code changes in the same pull request, in order to simplify the work for the programmer?
- or is it reasonable to expect the programmer to split their changes up into two (or several) PRs, one for functional changes, and one for refactorings/cleanup?
One smart person summed it up perfectly:
“For me, with my reviewer hat on, I want your code to get live as soon as possible, after being able to easily review it to make sure it does what it says it’s going to do. If it looks like I’m going to have to work out what’s changed in the middle of a 200 line section that’s been moved from one file to another, with a whole bunch of other minor formatting tweaks in the middle of it as well, then it’s going to take me longer to set aside the time to effectively review it.
With my PR raiser hat on, I want my code to get live as soon as possible. So I’ll make the PR as small and as easy to review as possible. And yes, I will often break it up into multiple PRs, with other things I’ve noticed whilst actually doing the thing I came here to do in separate PRs.
I just want good code to go live.”
Wonderfully put.
(Photo by Carsten Mueller from FreeImages)