(A workflow that preserves those commits requires actually having useful commits, and obviously if you have PRs with commits like "fix it" and "fix it more" then it might as well get squashed.)
(A workflow that preserves those commits requires actually having useful commits, and obviously if you have PRs with commits like "fix it" and "fix it more" then it might as well get squashed.)
If that's the state of developer proficiency with version control then, ideal workflows aside, we need a way to prevent those noise commits from ending up in master, and the easiest way is to squash with every pull request, as part of the "merge" automation.
What, you're going to let anyone waste time on tabs versus spaces?
Programmers sometimes have to decide stuff. That's okay.
"When appropriate" is the key here. That's asking people to make a decision, take on additional cognitive load, which does not have a clear deliverable or success criteria. In practice the default is what happens.
As I say to my children on the reg, "What's the best way to clean up a mess? Don't make it in the first place."
You're presupposing that this decision is worth making for programmers in the first place. I don't see why that ought to be true.
I don’t mind squashing either, unless I’m being really intentional or rewriting my history my intermediate commits couldn’t be reverted without leaving stuff broken (totally a me problem of course).
“try-again-something5” doesn’t cut it but “$ticket-at-least-five-words-here” does.
I find that the commit history tends to grow viciously for anything I've been involved with. And I fail to see the benefit of amassing that amount of detail once you are past the stages where each individual commit is reversible (or even interesting)
So, for a project that runs for, say, three months, the commits of the first few weeks aren't really very interesting or valuable at all at the end of the period. Just hard drive space being eaten up. YMMV.
Right now I have a full clone of a pretty large monorepo dating back almost nine years, and the .git dir is less than half of the total space. Sparse checkouts and shallow clones can make clown car hardware sort of work, but I do not want to go back to the pre-git days and try to work without full history to conserve 0.008 TB of SSD. We spend more than that on coffee.
> make clown car hardware
I'm not a native English speaker so this appears to me as a bit confrontational and/or an attempt to ridicule me or the points that I am making? I do not work with clowns, cars, or hardware.
> full history
I can see one benefit, and that is the case of sensitive software for special use (eg govt. mil, etc). Here, I agree that in the case of a vuln being discovered it could be a good thing to go back and trace the origin. So, I'm not opposed to keeping history, it's just not relevant to any particular extent for the types of tasks I do in my current $job.
My stance is more like, if I'll never use any of this stuff, why keep it at all? It's not about costs it's just keeping things simple
The problem with “fix it now” is that I didn’t know for sure that our behavior was wrong, I just knew that a consumer of our microservice had begun alerting on errors. I had to find out whether this was a mistake (which maybe I could safely fix) or important and intended (and any change must be negotiated with other consumers). It comes with having an old, complex system with a lot of dependencies and without exhaustive documentation.
Small PRs are better since they're easier to review.
For this reason I don't bother with Github and the like and just use Gerrit ;)
PRs often have a lot of overhead. They need a separate branch, CI jobs need to run, there are more notifications for everyone, separate approvals, etc.
Sometimes there's a need for keeping separate commits if they're all related to a single change. Proposing them all as part of the same PR helps maintaining that context while reviewing as well. Reviewers can always choose to focus on individual commits, and see the progression easily.
Sometimes it does makes sense to squash a PR if the work is part of the same change, but the golden rule of atomic commits always applies. Never just blindly squash PRs.
In fact, if the PR is messy and contains fixes and changes from the review, and the PR should have more than one commit, I go back and clean up the history by squashing each change to its appropriate commit. `git commit --fixup` helps with this, so I also prefer addressing comments locally rather than via GitHub's UI. Then it's a simple matter of running `git rebase --autosquash`.
BTW, thanks for all your open source work. <3 You're an inspiration!
...
> CI jobs need to run
A step is only revertible if the previous state has passed CI, and as you noted that only happens if it is a separate PR.
The only reason to split a PR into separate commits it to make it easier to review and understand. But if it's so big you need to do that, it should be separate PRs anyway really.
IMO the only time you should ever preserve branches when merging them is if they're long-lived ones that multiple people have worked on and the commits in them have passed CI.
The problem with that in practice is that the commits often depend on each other and then you have a choice of:
- serializing the PRs, i.e. only have one PR outstanding at a time, which increases your development latency; or
- no native UI for tracking how the PRs relate to each other
Yes, there are people trying to hack around the second problem. Still, the story there is far from great.
The CI thing is a real issue, I'll give you that. Though it is obviously also solvable, if the will for it were to exist.
I don't see a significant downside to this. It doesn't affect my development latency - if the commits depend on each serially other then you have to wait for them to be reviewed in order whether or not they are separate PRs.
I do agree that GitHub/Gitlab don't support dependent PRs very well. You pretty much have to wait for one to be merged before submitting the next. Not a big problem in practice but it could definitely be improved.
For example you have PR 1 and PR 2 (based on 1). Squash merge PR 1 into main. But while 2 is under review someone merges in 3 into main and you need to bring it into 2 to resolve some conflicts. You’re now hosed because the changes done in 1 are now present TWICE. First in 1’s commit, and also in the squash commit. Now you’re resolving all of 1’s changes as merge conflicts.
This may sound like a contrived examples but it has happened to me EVERY time I’ve worked somewhere that demands squash commits. And this is one of the reasons I hate squash commits.
“Makes the history prettier” vs “Rewriting the actual commit history”. Telling the truth > pretty.
Git history isn't "the truth" until it's shared. There's absolutely no issue rewriting history history for your own edits that nobody else is using yet. You do that every time you press undo!
Unfortunately, that part doesn't work if you let GitHub do a squash, because then the commit on main has no corresponding commit on (the branch of) PR2.
When cherrypicking the commits corresponding to PR1 during the rebase, the merge algorithm will notice that the change is already there and notify you of the empty commit if the change is completely identical. But if it isn't, that falls down.
It's still not too difficult to recover, but it's annoying.
Did you try it? It does still work. Git will detect that the changes are identical and drop it, even if the commit hash is different.
This is of course a balancing act. Which is my point. Sometimes it makes sense to split things up into multiple PRs. But sometimes it makes sense to fatten a PR a little bit with multiple commits. You can't just say "small PRs are better." Size is but one dimension of what makes a good PR.
This is why I personally use both "squash & merge" and "rebase & merge." If a PR has a bunch of commits but is really just one logical change, then I'll squash it. But if a PR has thoughtful commits, then I'll rebase it and do a fast-forward merge.
My bottom line is that I try to treat the source history as well as I treat the source. The source history is a tool for communicating changes to other humans, both for review and for looking back on. Squash & merge has a place in that worldview, but so does rebase & merge.
Having to fix the same merge conflict for each of your commits is one of the leading causes of developer burnout :D
Regardless, `git rerere` is supposed to solve that problem, but I don't do enough conflicting merges to be intimately familiar with it in practice.
It's possible the tooling could handle that case much better, but until it's sufficiently better that it's as simple as `gh pr create` by the author and one click of a merge button (or equivalent "@somebot merge") by the reviewer, that's still too much.
Also, a lot of people (myself included) write really crappy commit messages that don't tell the whole story behind a change. This is another reason why falling back on the PR has been valuable for me.
(Merging has the same problem, so I squash frequently and then rebase.)
Also having many commits does not means it's going to be easier to revert / fix than a single big one.
Can you give an example? I don't even think I understand what you're saying. You control your overall summary, so it's up to you to make it useful.
I would cite clarity as my reason for squashing! I think most people are just bad at organizing (& naming) their commits, so they make A LOT of vacuous ones, crowding out the forest for the trees. It's never helpful to see the git blame saying things like "Addressed PR comments" or "Resolved merge conflict", etc.
I do prefer a merge commit when the author has done a good job with their own commits, but that's rare. In all other cases, squashing is greatly preferable to me.
> Can you give an example?
Sure. Here's a common pattern I've used and seen others use:
Commit 1: "Introduce a helper for XYZ", +35,-8, changes 1 file. Introduce a helper for a common operation that appears in various places in the codebase.
Commit 2: "Use the XYZ helper in most places", +60,-260, changes 17 files. Use the helper in all the mechanically easy places.
Commit 3: "Rework ABC to fit with the XYZ helper", +15,-20, changes 1 file. Use the helper in a complicated case that could use some extra scrutiny.
I don't want those squashed together; I want to make it easy, both in the PR and later on, to be able to see each step. Otherwise, the mechanical changes in commit 2 will bury the actual implementation from commit 1 in the middle (wherever the file sorts), and will mask the added complexity in the commit 3 case.