6 ms·
I'm a big fan of the rebase workflow, but not of squashing. I wrote it as several separate commits for a reason: documenting each step, making each step reverti
by JoshTriplett 3y ago
I'm a big fan of the rebase workflow, but not of squashing. I wrote it as several separate commits for a reason: documenting each step, making each step revertible, separating refactors from from semantic changes, etc. Squashing makes the diffs much more painful, such as by burying an important change in a sea of mechanical ones when they were originally in separate commits.
(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.)
- jmharvey 3y agoI'm a fan of the workflow where the PR gets squashed in the upstream git repo, but the individual commits are preserved in the PR in the code review tool. I feel that Phabricator handles this well.
- easton 3y agoGitHub and Azure DevOps also do that, you just need to know where to look. 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).
- hamburglar 3y agoIntermediate commits being snapshots of “broken” state isn’t a problem at all. When I quit for the day, I commit, broken or not, and pick it up in the morning. I want to be able to drop my laptop in a puddle and still pick up where I left off when I get a new one.
- iimblack 3y agoI think the problem is putting those broken commits into the trunk. Ideally you want to clean up your commits so if you need to revert you don’t accidentally break your build and so reading thru history isn’t awful.
- hamburglar 3y agoNobody anywhere suggested putting broken commits in trunk. This is why branches exist.
- usrusr 3y agoBut does that still lose the source commit long term? What I'd love to have is a mechanism that keeps references to the pre-squash commits at blame granularity, allowing one to dig deeper into the commit messages associated with a given line. Kind of like a sourcemap, but for squash instead of transpile.
- 91edec 3y agoWhen you squash merge on github the new commit references the old PR. If you don't delete branches on merge you would keep the commit history on that branch, but then you have to battle with branchs persisting forever.
- cplusplusfellow 3y agoBranches are mostly free, so this isn’t a problem if they are properly named. “try-again-something5” doesn’t cut it but “$ticket-at-least-five-words-here” does.
- TheCleric 3y agoBranches are not cognitively free. Searching through the haystack of hundreds of branches to find a particular needle is a pain.
- cplusplusfellow 3y agoYou’re translating the problem from : searching through branches that are named according to their ticket and what they are meant to accomplish to: complex and not-context-free git bisect.
- jacobegold 3y agoYou don't lose it long term if you're using GitHub PRs — GitHub keeps the "reflog" (quoted because I imagine their implementation may not actually use reflog) of the branch indefinitely, even after force pushes. Graphite (built to replicate the Phabricator workflow) enables viewing and diffing these versions. (disclaimer, I helped build this)
- klipt 3y agoIf you want to preserve separate commits you can just make them separate PRs? Small PRs are better since they're easier to review.
- tazjin 3y agoThe problem there is that toy systems like Github's review tool don't have good flows for stacking dependent changes, and then people end up not bothering. For this reason I don't bother with Github and the like and just use Gerrit ;)
- imiric 3y agoThere are many reasons for having several commits in the same PR. 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`.
- burntsushi 3y agoI do the same. Given that you didn't mention git-absorb[1], I wonder if you've tried it. If not, you might be very happy to discover it. [1]: https://github.com/tummychow/git-absorb https://github.com/tummychow/git-absorb
- imiric 3y agoInteresting. I'll give it a try, thanks. At first glance it reads like too much magic that I'd have to undo manually, but if it works reliably, it would save some time. Though usually it's not much work to figure out which commit to reference in `--fixup`. In short histories, I usually just use `HEAD~<n>`, instead of hashes. BTW, thanks for all your open source work. <3 You're an inspiration!
- aeternum 3y agoRebasing would be so much better if Git just had better defaults. Having to fix the same merge conflict for each of your commits is one of the leading causes of developer burnout :D
- jauntywundrkind 3y agoYeah it can be a bit of a pain. If you have long running branches or do keep hitting having to re-resolve, getting good at git rerere is recommended; "reuse recorded resolution"! https://hn.algolia.com/?q=git+rerere https://hn.algolia.com/?q=git+rerere
- cplusplusfellow 3y agoI’ve known developers who left the company over the rebase mandate for this exact reason.
- o11c 3y agoRebasing generally doesn't require repeated merge conflicts though? Since the formerly conflicting commit disappears and the non-conflicting one is now baked into a linear history. 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.
- mrkeen 3y agoSquash before rebase?
- jen20 3y agoIf you actually have commits which stand alone - i.e. the build succeeds, the tests pass etc - I see no reason not to land them altogether with a rebase. What I object to is commits which do _not_ meet those criteria, and make life much harder for the next person who has to spelunk through the history trying to work out what has happened and why.
- JoshTriplett 3y agoWe're in full agreement there. If you run an "every commit should compile" project, rebase. If you have "fix" and "another fix" commits stacked atop the original in a PR, by all means squash them all.
- Thaxll 3y agoimo you should always squash,when merged pr / mr should act as a single unit not many commits. Also having many commits does not means it's going to be easier to revert / fix than a single big one.
- kevingadd 3y agoHistorically any time I try to rebase a branch with more than a few commits in it, it effectively fails unless I squash it because resolving all the conflicts for every commit would take me over an hour. Maybe this is just a 'many contributors' problem though, since our repo lands many large PRs each day. (Merging has the same problem, so I squash frequently and then rebase.)
- feanaro 3y agoHow does merging have the same problem? By definition, merging only requires you to resolve the conflicts once, at the point of the merge commit.
- yetanother12345 3y agoSquashing is nice IMHO, and even a must after a while. For one recent very small project a squash of the commit history reduced the storage from tens of kilobytes to a few hundred bytes total. Orders of magnitude. That was a very small project, so imagine the storage space savings for larger projects. 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.
- erik_seaberg 3y agoReal example: I had to mitigate an outage in the middle of the night, and I found the root cause in ten lines of code. I needed very badly to “git blame” that code and find a specific commit message from three years ago and its author (a former colleague), to figure out what he had been trying to do. 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.
- JoshTriplett 3y agoExactly. Storage space is much cheaper than human time and brainpower.
- yetanother12345 3y agoOk, so it appears we just approach some problems differently. If I had to work in the middle of the night and found a problem spanning ten LOC my procedure would be "fix, commit, back to sleep". I would never feel any need (nor have the time) to investigate the origin of the problem, much less feel a need to "git blame" anyone. Likely I would not even look at the commit history as (to me) historical code is not really relevant from a current-problem-solving standpoint. But that's just me I guess. > 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
- blakesley 3y ago> Squashing makes the diffs much more painful, such as by burying an important change in a sea of mechanical ones when they were originally in separate commits. 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.
- JoshTriplett 3y ago> > Squashing makes the diffs much more painful, such as by burying an important change in a sea of mechanical ones when they were originally in separate commits. > 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.
- trevor-e 3y agoIt sounds like you're more describing a stacked PR workflow. I achieve the same thing using stacked PRs and can still have a bunch of the "fix stuff" intermediate commits that get squashed away, because who can really predict whether everything will pass CI in the first attempt. :)
- JoshTriplett 3y agoI don't want to have a stack of PRs where each one depends on the previous, where each PR needs to justify itself separately while at the same time being interdependent and ordered. That adds cognitive overhead and makes it take longer to get merged, if it gets merged at all. 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.
- alphazard 3y agoMost developers I've worked with think of Git as a CLI front-end to GitHub. They aren't writing commits with the intention of showing their work, they are using commits to save their work before signing off, or to trigger CI. They aren't proficient enough with Git to go back over these and clean them up, as would be expected in a project like the Linux kernel. 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.
- burntsushi 3y agoWhy do you need to squash & merge literally every PR? Why not use squash & merge when appropriate and rebase & merge otherwise? That's what I do for my projects. Most contributions coming from others tend to get squashed & merged, where as I'll rebase & merge my commits. But I'll sometimes rebase & merge PRs from other contributors who have broken their change down into thoughtful commits.
- kdmccormick 3y agoThis is it. The real answer to this whole debate is "it depends" but for some reason that doesn't seem to satisfy people.
- rokkitmensch 3y agoThe effective management approach (in opposition to the limp-souled "servant leader" anemia) is to make decisions for your team to eliminate stupid work. What, you're going to let anyone waste time on tabs versus spaces?
- burntsushi 3y agoYou've presupposed that this is all about stupid stuff in the first place. Moreover, mixing tabs and spaces for indentation leads to sadness. But squash and rebase are perfectly compatible with over another. Programmers sometimes have to decide stuff. That's okay.
- HeavyStorm 3y agoMe too. I think git blame is the ultimate documentation tool - you can have a description for each block of code, tied to who and when it happened. If you squash, all the sudden you have a simple explanation for hundreds or thousands of lines.
- nunez 3y agoIf you're using GitHub or gitlab and merging through pull requests, I've found that these commits become duplicative and, given GitHub's rich comments-based collaboration UX, somewhat lossy. It is much easier/more valuable for me to view a commit off main that points to the PR that brought it in and the discussion that took place (along with the original commit in that branch) than to see the individual commit rebased into main without context. 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.