Relatively common practice where I work. What do you do instead?
If you’re very diligent about making good atomic commits that always pass all test that can work, but I find that squashing PRs (and PRs being relatively small) is a very good trade off
I've recently started to like squashing after usually preferring rebase merges, because with squashes I have an easy option to rewrite the commit message and edit out all of those meaningless "fix typo", "improve" and "now actually working" lines...
And why couldn't you rewrite the commit messages while rebasing?
EDIT: oh, you mean from the GitHub webUI, I presume. Got it.
Yes, exactly, while reviewing and merging other peoples PRs. Technically I could still rewrite in other ways, but while squashing in the web UI was the most comfortable workflow.
So the idea is really "what's the point of having a history full of broken state?".
> It must be impossible to understand commits' diffs with the changes all squashed together.
This would be a hint that your PR was too big and addressing more than one thing.
And the answer is that you don't; each commit is individually testable and reviewable. Changes requested by reviewers are squashed into the commits and then merged into the project. Unfortunately, while the git command line has "range-diff" to ease review with this workflow, neither GitHub not Gitlab have an equivalent in their UI.
Of course, if your workflow is different, then... well it is different. Doesn't make the "squash workflows" irrational.
Disclaimer: I don't squash PRs.
How does this work in practice? Is every single atomic commit reviewed by someone? When do they review each of those commits? How many commits typically go into a PR?
> Changes requested by reviewers are squashed into the commits and then merged into the project.
So a reviewer finds the appropriate commit that their comment applies to, and then changes the actual commit itself? Who is the author of the commit at that point?
I'm trying to understand what you're talking about, because you seem to have something figured out, for a problem that every team I've worked on struggles with.
1) yes 2) when a PR is submitted 3) it can be a lot for a huge project-wide refactoring, but generally I would say 1 to 5 is typical and up to 20 is not strange.
> So a reviewer finds the appropriate commit that their comment applies to, and then changes the actual commit itself?
No, the author applies the requested change and force-pushes once he has gotten all the requested changes applied.
> because you seem to have something figured out
Thanks! But it's not me—it's how Linux has used git from the beginning, for example. In fact it's the only workflow that is used by projects that still use email instead of GitHub/Gitlab PRs, but (trading some old pain with new pain) it is possible to use it even with the latter. The harder part is marching the review comments to the new patch, which is actually pretty easy to do with emails.
It's quite some work and there's some learning curve. But depending on the project it can be invaluable when debugging. It depends a lot on how much the code can be covered by tests, in particular.
I rebase commits so they don't break the build but the history remains clean and incremental. Selective fixups and so on isn't the same as squashing everything into a single commit.
> This would be a hint that your PR was too big and addressing more than one thing.
I don't think so. Sure, that can be true, but squashes can also simply lose vital history. Suppose you remove a file and then replace it with code copied and modified from another file. If you then squash that, all Git will say is you made a massive edit to the file.
Sure, and that's fine. The idea of the squash workflow is that they don't expect that. It's just different, and that's the rationale behind it :-).
> all Git will say is you made a massive edit to the file.
Which IMO is exactly what happened in this case xD. But again... whatever floats your boat, I was just talking from the point of view of a squash workflow.
Of course it's got the same end result as doing an interactive rebase & combining all the in-progress commits into a single reviewable unit of change with a good commit message, but it's a bit more automatic.