I couldn't care less about the commit messages on a PR, I care about the diff. as long as the messages are cleaned up in the squash and merge with main
If it's large I want to be able to look at the commits in isolation and understand the work in the logical chunks that made sense to the author.
If what made sense to them is temp, checkpoint, temp, temp, undo the temp, fix test, try this, that didn't work try other thing, maybe?, temp, fix test - then I don't stand a chance. Recently I reviewed one that had multiple 'rebase' commits, I have no idea.
- But sometimes I want to have a few distinct (isolated commits for my PR)
- Then make the PR small
- But I have several changes
- Then make several small PRs
We keep coming back to “just make more PRs”. Which is curious, given that GitHub doesn’t even support dependent PRs. The thing you need immediately when your PRs start depending on each other (like refactor X before implementing Y, where both touch the same code).
But I can’t come to any other conclusion than that this is because of the over-focus on PR as the one and only reviewable unit. Thanks to GitHub.
I can’t really get on the small PR train. A PR has overhead and I often want to do small changes that I only happen to notice are necessary when I am in the middle of doing that one PR.
That begs other questions. The relevant reviewer isn’t available until Wednesday. Do I start on the next tasks in dependent branches? The act of making all those branches is a large part of the inherent busywork. And then I need to remember to get back to them when the reviewer gets back.
The benefit of all these small PRs is yet to be revealed.
Is the dialogue here that you ask questions that makes the workflow more and more constrained and then when X factors are fixed it becomes obvious that the workflow works great?
Of course I start on work that I (not the reviewer) can work on right now.
3. I'd rather the PR were whatever size it needs to be to entirely do the thing it's supposed to do (and nothing else) than conform to some arbitrary size requirement.
But that's the thing, in most cases there's no review step anymore after the squash/merge has been made; while it's in a branch / MR, you can still edit the commit messages and content, but in most cases the squash and merge is an atomic, unreviewed step. Of course, maybe the merge commit should be generated, or should be filled in and reviewed as part of the main code review.
Yeah, I’ve noticed that many around the Internet just comment with “the squash and merge” as if it’s a given that that’s the strategy that is used (even mandated).
