13 ms·
Squash merges cut down the noise considerably.
by recursive 3y ago
Squash merges cut down the noise considerably.
- mablopoule 3y agoI actually hate squash merge because of all the noise it adds. Sure, the commit graph looks nicer, but it come with a terrible loss of information when doing git blame. I'm a big proponent of rebase and squash if it helps to make a commit more coherent, but we use squash merges by default in the current project I'm working on, and I die a little bit each time I try to understand what changes were related to a line when tracking down a bug.
- nerdponx 3y agoThis is the big one for me. destroying Commit information just to keep the graph tidy is a bad idea in my opinion. It would be better if Git provided better tools for filtering the log, e.g. providing some mechanism to elide commits from parents of any merge commit other than the 1st.
- diek 3y ago> destroying Commit information just to keep the graph tidy is a bad idea in my opinion The commit information I see when telling teams to squash their branches on merge is not valuable. * "fixing whitespace" * "incorporate review comments" * "fix broken test" * "fix other broken test" (note, the broken tests were broken by the changes in the PR) As soon as that PR is merged those commits are worthless. And there are branches with dozens of those "fixing X" commits that would otherwise pollute the commit graph.
- lelandfe 3y agoThose commits would be the bathwater one casts out alongside the useful commits in using squash merges.
- diek 3y agoIf the useful commits are the "baby" in your bathwater analogy, all the useful information in those commits is in the squashed commit. This assumes a branch being merged in represents one logical change (a feature/bugfix/etc) that is "right sized" to be represented by one commit.
- mablopoule 3y agoYes, but now it's mixed with the bathwater, and now morph into another metaphor as it become the needle in the haystack. It's okay to have 'low information' commits one can easily ignore in your history, as long as the 'high information' ones stay readable and coherent.
- gray_-_wolf 3y ago> * "fixing whitespace" * "incorporate review comments" * "fix broken test" * "fix other broken test" Things like this should not be standalone commits though, they should be incorporated into the previous branch by amending the original work. It takes some effort to have a useful git history, it does not just happen on its own.
- lesuorac 3y agoSounds like six vs half-dozen. Why does it matter if somebody amends vs squashes?
- gray_-_wolf 3y agoIt does not matter if you have one commit. If your change is split into few commits for increased readability, in that case it does matter. Do you really believe that if, for example, this change to btrfs filesystem https://lore.kernel.org/linux-btrfs/cover.1699470345.git.josef@toxicpanda.com/T/#t https://lore.kernel.org/linux-btrfs/cover.1699470345.git.jos... would be squashed, nothing of value would be lost?
- noahtallen 3y agoYou can very easily rewrite your commit message on GitHub when squash merging. Since the organizations I work exclusively use squash merge, I often just update the commit to be more valuable, listing the important changes it contains. (And of course the PR in GitHub will contain the commit history of the branch that was squashed, as well as any discussion.) IMO, this is a lot simpler and easier to do than rebasing your branch to have a flawless history.
- sodapopcan 3y agoI rather strongly disagree here. Having whitespaces mucks up commit, causing you to lose focus of what's actually important. I have `git blame` aliased to `git blame -w` which ignores whitespace-only changes. You can also reblame when you come across this formatting commits.
- trevor-e 3y agoYep, intermediate commits on a branch tend to be completely worthless. I'd much rather have "git blame" point to the commit that contains the entire change together.
- dieselgate 3y agoAgree strongly, it's nice in theory to view the intermediate commits but in practice have never needed to look at them
- 26fingies 3y agoYou can usually see that in whatever tool youre using anyway. Blame -> find the PR -> see commit history.
- gray_-_wolf 3y agoYou mean like for example `git log --first-parent`?
- nerdponx 3y agoTIL, thank you! Now I know for a fact that squash-mergers have no excuse and can brandish the man page at them.
- WorldMaker 3y ago`--first-parent` also today works for blame and bisect.
- Izkata 3y agoPeople in general just have no idea how much version control is able to do. For one example try running "git help log" and just tap page-down a few (dozen) times to get an idea what's in there. For another example, you know how people hate aligning code vertically so much that linters don't allow it nowadays, the primary reason being that if you have to change the spacing then the diffs will identify far too many lines as having changed? Both git and svn have options to ignore whitespace changes: git diff -w svn diff -x -w
- johnsbrayton 3y agoI do squash merges but keep the feature branches. So after determining that I made a change as part of a big pull request, I can then look at the commit/blame history for the pull request source branch if necessary.
- matijsvzuijlen 3y agoThis means having to keep these branches around cluttering up everything, and makes git bisect a lot more complicated.
- mvdtnz 3y agoI'm guessing you don't work on large projects. This would create an outrageous amount of noise in a busy repository.
- skybrian 3y agoThis all depends on the project. Sometimes you don’t look at history all that much. Sometimes the loss of information is acceptable.
- Izkata 3y agoIf you don't look at history much, why would you care about keeping it "clean"? Just keep the truth of the changes in the history for those of us who do use it, and you can continue ignoring it.
- diek 3y ago> I die a little bit each time I try to understand what changes were related to a line when tracking down a bug A change/feature/bug is a branch, which is squashed into a commit on your main branch, right? So your main branch should be a linear history of changes, one change per commit. How does that impact the ability to git blame?
- joshribakoff 3y agoBecause sometimes a PR touches more code than a single commit, and you lose the more granular context surrounding the more granular changes. You can always ask git to make the log more coarse, but once you “destroy” the granular history it is for all intents and purposes gone.
- js2 3y agoBecause unless it's the most trivial of features, you'll break it up into smaller commits which each explain what they are doing and make reviewing the change easier. As a simple example, I recently needed to update a json document that was a list of objects. I needed to add a new key/value to each object. The document had been hand edited over the years and had never been auto-formatted. My PR ended up being three commits: 1. Reformat the document with jq. Commit title explains it's a simple reformat of the document and that the next commit will add `.git-blame-ignore-revs` so that the history of the document isn't lost in `git blame` view. 2. Add `.git-blame-ignore-revs` with the commit ID of (1). 3. Finally, add the new key/value to each object. The PR then explains that a new key/value has been added, mentions that the document was reformatted through `jq` as part of the work, a recommends that the reviewer step through the commits to ignore the mechanical change made by (1). A followup PR added a pre-commit CI step to keep the document properly linted in the future.
- diek 3y agoIn general I agree with you, there are absolutely times where you want to retain commit history on a particular branch (although I try to keep the source tree from knowing about things like commit IDs). I would argue that those are by far the minority of PRs that I see. As I mentioned in another comment, _most_ PRs that I see have a ton of intermediary commits that are only useful for that branch/PR/review process (fixing tests, whitespace, etc). Generally the advice I give teams is, "squash by default" and then figure out where the exceptions to that rule are. That's mainly because, in my opinion, the downsides of a noisy commit graph filled with "addressing review comments" (or whatever) commits are a much bigger/frequent issue than the benefits you talk about. It really depends on the team.
- WirelessGigabit 3y agoMe too. I care a LOT about provenance. And squash merge completely breaks that. When my branch is up to date with `main` I can build an artifact, fast forward merge that branch into `main` and RETAIN the artifact, and merely update its tags to mark it as `merged` in. With a squash I lose that information. Now, GitHub does not allow me to do a fast-forward merge but I can still trace the 2 commits that are the parent of the resultant merge, and find the artifact based on that, and retag.
- Lio 3y agoGit blame confuses people even without squash merges. I've seen people forget to go back more than one commit and then blame the person who last indented a file instead of going back to the commit that actually wrote the code many times.
- erik_seaberg 3y agoWe shouldn’t tamper with code we don’t actually need to fix, it’s not a good use of time and it makes history less useful. Just because it doesn’t look like I wrote it doesn’t make it wrong.
- Lio 3y agoI’m thinking of situations where the surrounding structure of the code has been changed to correct a problem. That’s done by an automated tool. Correction of indentation is just a byproduct. I don’t consider that “tampering”.
- deleted 3y ago[deleted]
- WorldMaker 3y agoYou can deal with that through tooling. In a lot of my work I call those types of automated tool commits "wrench" commits personally and even have a simple shell script to help automate committing them. In my case I prefix the command line with a wrench emoji. At that point it's very obvious in git blame that if a line starts with a wrench it was last touched by an automated tool of some sort. You can also very easily at that point grep your git log for wrenches to dump commit hashes into a git-ignore-revs file and automate that part too so that those commits don't even show up in git blame at all.
- sodapopcan 3y ago> I've seen people forget to go back more than one commit and then blame the person who last indented a file instead of going back to the commit that actually wrote the code many times. I default my `git blame` to `git blame -w` which ignores whitespace commits. Though knowing how to jump back commits should be required knowledge.
- paulddraper 3y agoSquash merges are rebases.
- recursive 3y agoJust when I thought I was starting to understand git...
- Izkata 3y agoRebase essentially means "create new commits out of old commits", the original use being to "move" a set of commits from one branch to another (think of the name as meaning, to change the base these commits started from). There's a few special cases that have their own names, a common one is when you amend a commit - to do that manually you'd make a new commit, then use interactive rebase to squash the two commits together into a new one (or, use the "fixup" command available in that tool, which is a squash that automatically picks the first commit message instead of asking for a new one). Squash merges will squash a whole branch into a single commit, rebasing it onto the target in the process, and then fast-forward the target to the new commit. It's a tightly controlled use of rebase, and can be thought of a bit like how "for", "foreach", and "while" loops are a tightly controlled use of "goto", an abstraction built on top of a far more flexible tool.
- ksenzee 3y agoOnly metaphorically, maybe. You can squash merge in lots of cases where a rebase will fail.
- Izkata 3y agoThey are essentially rebase+squash, despite the name. There is no actual merge taking place. And for that matter, you'd manually do a squash with the interactive rebase tool anyway ("git rebase -i").
- ksenzee 3y agoImagine a feature branch where someone has been keeping it up to date by merging main into it regularly. Now the feature is ready to go into main. You can easily `git merge --squash` that branch into main. You can likely do the same thing manually (as you point out) by running `git rebase -i` if you squash all the commits in the branch. But you’ll never manage to do a genuine rebase, where every commit in the branch gets turned into a clean non-merge commit onto main.
- deleted 3y ago[deleted]
- dahart 3y agoWhat I think I see these days is squash merges being used lazily to avoid having to do anything to build a clean history with clearly semantically delineated commits. Squash merges are good compared to an alternative where people check in super messy noisy branches, but they unfortunately have a big downside because squash merges can make bisecting and history spelunking more difficult, when the branches that are squash merged were big.
- sidlls 3y agoWhat is a "semantically delineated commit"? What is a "clean history"? Why are these two things important?
- FeepingCreature 3y agoNot parent: there are technical commits, such as "fix review", "fix jenkins", "fix typo" etc. Those don't delineate a particular feature but a fix for a problem that arose from the workflow. This ends up with a history of "big feature commit that is wrong in three trivial ways" + "fix 1" + "fix 2" + "fix 3". Of those, "big feature commit" is the important one, but "fix 3" is the only working one. This is clearly silly; you should pretend you were perfect from the start and squash "fix 1" through "fix 3" into "big feature commit". Your typos and brainfarts are not of historical relevance.
- sidlls 3y agoPerhaps I'm missing something, but I don't see how your comment answers my questions. Do you mean that a "clean history" is one without "fix 1", "fix 2" and "fix 3"? Or is that a "semantically delineated commit"?
- ruds 3y agoA clean history is one where there is a single commit, "big feature commit", that produces a worktree that is the same as the one produced by "fix 3" in the "unclean" history.
- dkarl 3y agoI think squash merges are a last resort heavy-handed tool for dealing with developers who refuse to clean up their commit history before merging. Most developers can do better by hand. Git history should tell a simple, understandable story of each change. For example: 1) refactor existing code, 2) add feature. Or 1) add missing tests, 2) refactor existing code, 3) add feature. But since you're working on the fly with imperfect knowledge, it doesn't happen in such neat steps. Refactorings and behavior changes end up interleaved in your raw git history, so you need to do a little bit of cleanup by hand in order to present a simple story in the commit log. Of course if you have developers that don't do that and instead merge dozens of commits that just say wip, wip, wip, lol, fml, wip, wip, lol, yolo and you can't fire them or get them to change, then squash merges ftw.
- bogeholm 3y agoI ser it the other way around - why spend time on a ‘nice’ commit history in a (smallish) feature branch when you can squash merge later. I prefer one commit to main per feature, a long with a good description on the GitHub PR. Sometimes I’ll branch out from a feature branch for the occasional and infamous ‘get CI working’ round of 10 one-line commits though, to not make it too muddy.
- ufo 3y agoPrecisely, you want to keep it about one commit per feature. I think the parent comment was worried about monster merges that squash many features together.
- marcandre 3y agoThe classic example where this fails is when needing to revert something. An atomic commit for the migrations + some atomic commits for the implementation mean you can easily revert the implementation, and leave the migration intact (as should be) and add a reverse migration.
- gwright 3y ago> why spend time on a ‘nice’ commit history in a (smallish) feature branch when you can squash merge later. Several reasons: * facilitates much better code review discussions * enables use of git bisect to locate bugs * allows for informative commit messages associated with the changes * communicates clearly to future self about why changes were made
- jawns 3y agoMy rule of thumb for commits is that they should be of a size and scope suitable for cherry-picking. So, maybe I'm working on a small feature that entails three changes, and each of those three changes is useful in and of itself and could conceivably be cherry-picked by others. I would create three separate commits, generate a PR with all three, and merge in the work. Sure, I could squash merge and end up with one merge commit encompassing all three changes, but now none of those three changes is cherry-pickable.
- sodapopcan 3y agoNo one is mentioning: $ git log --merges Now you can see your features in a nice history and also have added benefit of seeing intermediary commits. Pro tip: merge commits aren't required to use the canned "Merge branch into..." message, you can give it any message you want, such as "feat: ..." or whatever your convention is. I hate that branch squashing has become something of a defacto. I actually do rewrite my history and often add context to my commits. `git blame` can be an incredibly useful tool to get context about a given small change. Getting a massive diff for a whole feature is much less so, especially since you can just look at the diff of the merge commit.
- Lio 3y ago> Squash merges cut down the noise considerably. They do but they have their own issues. e.g. having to delete local branches using git branch -D instead of git branch -d and getting the protection from deleting unmerged work. I still agree that on balance annoyances like that might still be worth putting up with for larger teams with mixed skill levels.
- mcv 3y agoThey also cut down the signal.