Squash merges cut down the noise considerably.
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.
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.
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 madeThis is really only viable if each intermediate commit on a development branch is intended to be bug free. If that's the standard you and your team work with, that's fine, but it's not usually my standard; in a development branch, I may commit things that don't even compile, let alone work, if it's a good point to commit.
IMHO the expectation is that each commit would 100% pass CI, so if you decided to extract some commits and merge that early you can. This is especially useful when a 6 commit PR is reviewed, and the first 3 commits are fine but there is more feedback on the last three. The reviewer can split the first 3 good ones out, get them merged and whittle down the PR to the remaining three. The subsequent follow up will be less.
IME team velocity goes up with this too, and it encourages small and easy to review commits like a Remove to be extracted and merged early.
Since PRs are always as large or larger than commits, I would much rather have a specific commit flagged than have to wade through the whole PR diff. If the PR is not familiar to me, I want to increase my effectiveness narrowing down the cause, so I can fix it faster.
In other news, our developers also create several small PRs every day but each is for an incomplete change that doesn’t stand alone so we’re never quite sure which features are finished in any given build, everyone keeps complaining about being interrupted to do code reviews all the time when the code reviews have no value anyway because they always just say LGTM :+1:, and we have targets that no more than 15% of commits should break production when CI/CD deploys them and that we recover fully within an hour each time that happens. If only there were something we could do to improve all this…
He was lead and could influence salaries, his opinion mattered. So, in real life, people rarely pushed back. That is not the same as us sharing the same opinions tho. I became more verbal about history not being useful online.
I rarely use history and prefer merge/squash, with automated CI tools, and tests. "Why" is kept in doc strings, comments, specs, and story tickets. Everything viewable in gitlab with automatic links. All this gets out of the critical path, every day.
I submit that, if your code is so complex that diagnosing a bug is a major research project rather than moving forward with a few extra/modified lines of obvious fix, then that is the problem to focus on.
Sorry, but I don’t buy that. By the same principle, there’s also no point in writing unit tests or defining static types or having code reviews, all of which require thought and extra work, yet can yield considerable dividends when done even moderately well.
I rarely use history and prefer merge/squash, with automated CI tools, and tests. "Why" is kept in doc strings, comments, specs, and story tickets.
The argument for a tidy history isn’t just about a different place to explain a change. It’s about presenting work in clearly defined, meaningful steps to other readers like code reviewers, or perhaps someone who found these commits later through `git blame` on a problematic line of code or `git bisect` after a regression. It’s about each commit representing a complete, self-contained change that could later be reverted, or cherry-picked or merged to another branch.
I submit that, if your code is so complex that diagnosing a bug is a major research project rather than moving forward with a few extra/modified lines of obvious fix, then that is the problem to focus on.
Some problems have a lot of essential complexity. The code to solve them necessarily has at least the same degree of complexity. Sooner or later, there will probably be a change to that code with an unintended consequence for something else. Keeping the code and its history tidy and systematic is, IMHO, how you avoid those investigations becoming major research projects.
As an industry we get paid primarily for 1) working software and 2) communicating with stakeholders.
Tidy yet inaccessible (to non-dev) construction stories are not on that path. I would argue unit tests et al are, to ensure #1.
No stakeholders? Put why into a readme, where it can be seen at a glance. Comments can reference docs.
Complexity must be broken down into bite-sized chunks for a solution to be feasible in the first place, reliable in the second. i.e. skull-size limits. If there’s any code I don’t understand I rewrite it until I can. With tests of course.
I see version history as an asset, just like the code itself, tests, developer documentation, the bug tracker database… None of these things are directly visible to end users under normal circumstances, but they are useful sources of information and organisation and collaboration that help developers to create the software that users do see.
To me, a repo with a messy version history is like code full of superficial comments, a test suite with high coverage metrics that still doesn’t exercise the most important functionality, a dev team where the only documentation is some auto-generated static site that reproduces what any decent IDE would show in real time anyway, or a tracker where all the tickets are vague one-liners. You can produce useful software despite those things, but why would you?
Also, I said/meant stakeholders not end-users. Ours definitely do write bugs, look at docs and generate db reports etc.
The main distinction is that things on that list have a high cost-to-benefit ratio to goals 1 & 2, where history maintenance does not. The cost is high and utilization isn't. Additionally it can't be used to communicate with anyone but developers.
Not true. I do not do those to have nice clean process. I do unit tests, because without them the code is unstable and it is hard to fix bugs without causing unrelated ones. If the code is super simple and unlikely to break, I don't do test. I like to use static types, because I am much faster when writing them. The code is more readable and I have less bugs. Now, I have seen both useless and useful code reviews.
But, in all of those cases, things are done because they beneficial impact in final code and speed of delivery. Beautiful git history does not have such tangible measurable benefit. Git blame and bissect work without it, you just need one more step once in a while.
I respectfully disagree. In my experience, a tidy history directly benefits both efficiency and outcomes of code reviews, speeds up investigations of both bug reports and sometimes general background before starting new development, makes development much easier in situations where changes may need to be isolated and deployed to specific environments (not all software is a web app using CI/CD…), makes it much easier to back out a problematic change without causing unnecessary collateral damage, and helps to verify which development has actually been completed and deployed to which stages/environments, which can be useful for general awareness around the team but is particularly important if you’re operating in any kind of regulated field. All of that in exchange for usually spending less time in `git rebase -i` than it’s taken me to write this comment seems like a bargain to me, but YMMV.
Not to mention quality is significantly higher now so you wouldn't want to refer to a granular history of crap anyway. Any time spent on that would have been completely wasted as I wipe out a thousand line file for a new one with a hundred lines because requests hadn't been invented yet and the original implementer didn't understand network protocols or how to use argparse and implemented it from scratch poorly.
If you can afford your first instinct to be reimplementing things from scratch, your understanding of the value provided by proper version control will be limited. Some of us work with constantly changing code developed by thousands of people from all around the world in projects that 15 years ago were migrating to git and that have tons of downstreams, and are thankful for maintainers and processes that keep their commit graphs useful.
Though that said, once you're comfortable enough with git you'll be thanking yourself for commit hygiene even when coming back to your few years old single-person codebases.
In my experience, developer's work consists mostly of gaining understanding of codebases. It's like being a detective. Writing new code happens too, but not as often and it's not as impactful (and usually can and should be handled by less experienced devs wherever possible). Among the most impactful things are single line changes that took a week to write, or a few dozen lines that took months. Rewriting existing code from scratch is something that happens only as a last resort and after very careful consideration. Maintaining some basic version control hygiene makes a whole world of difference in such work. Sure, you can live without it, but you can also live without docs, comments or tests (and sometimes have to - which makes you appreciate them when they're there).
This too, but there's another thing at play as well: many developers don't know git at all. They just memorized enough commands to let them do their work. They don't understand what they're doing, so they can't reap the benefits of the tool they use. You won't get much use of RAW photos if all you can do in a graphics editor is clicking "auto enhance" button.
That said, I've also come to the conclusion there's basically two classes of Git users: people who really understand Git and use it fully, and those of us who basically use it as a place to shove source code before quitting for the night.
At one company with a Giant Custom Enterprise App, I ended up occasionally acting as a historian for pieces of the company with bad communication/institutional-memory, ex: "Oh, the +5% Foo charge was because of a request 3 years ago by vice-president X, here's the ticket number, before that it used to be +3%."
In those circumstances--where the implementation is the source of truth for business process--a well-maintained stream of commit-messages become quite useful.
Either a short quip, doc string, or link to the full story is an accessible combo. Nothing is the correct choice for unsurprising code.
Noting each feature ticket that ever affected a line--or even just at the function level--is sort of like maintaining a few thousand incomplete micro-changelogs. Doing it "acceptably well" takes much more effort than grooming the commit history so that someone can click "show change history for selected lines" in their IDE.
Plus consider all the unnecessary noise it makes for people reading the code, or reviewing a PR.
So, yes seeing e.g. how "CSV export for class A" is implemented is a great guide for implementing "CSV export for class B".
Everything you need is in the most recent copy. Showing an old one invites errors for no benefit.
You would then typically supplement reading the commit with reading the current version of the affected code, but looking at the commit points you in the direction of the files and methods you need to look at.
Not to mention just finding the right commit months later sounds like more work than that already.
Personally, even if I were to make the history absolutely perfect, I never get the code right (interfaces etc), the first time. It might be hours or days before I'm 98% happy with the final implementation. Sometimes big refactor opportunities come to me months later, e.g. where I move code needed multiple times into a more central mixin location.
Aside, `git commit --fixup HEAD` is often better than `git commit -m "oops, one more thing"`, since it means you can easily `git rebase -i --autosquash`.
What I push out are atomic commits that make sense logically, not an external undo log of my text editor; squashing those on merge provides no benefit and only loses useful information. Squashing should happen before push, not on merge, and there's no reason to have buggy "intermediate" commits recorded in your central remote branch at all.
> This is really only viable if each intermediate commit on a development branch is intended to be bug free.
git rebase has an --exec option that allows you to run a command or set of commands for each commit in the branch. You could rebase your development branch before pushing it up for review and ensure each commit passes coffee linting and tests.
Hmm, I usually mark PR’s as draft until ready for review, and then I expect the discussion to be about the current state, not a previous intermediate state. Easiest with small PR’s.
> enables use of git bisect to locate bugs
Interesting. I know _of_ git bisect, but haven’t used it as part of my workflow. Have you found it useful to bisect commits on a feature branch (which, presumably, represents unfinished work)?
> allows for informative commit messages associated with the changes
I find using the PR title and accompanying info in GitHub or similar to be quite informative - that should convey the purpose of the change.
> communicates clearly to future self about why changes were made
See above. Perhaps we work differently, but I find it clearer to read a git history where each commit represents a single, complete feature/fix/refactor instead of intermediate steps.
> facilitates much better code review discussions
This can be done while adding code to the feature branch
> allows for informative commit messages associated with the changes
I'm assuming you consider individual commits to be the basic unit of change? This isn't always the case. Some products are not amenable to adding features fractionally
> communicates clearly to future self about why changes were made
You can do that with a squash-merge too!
I've noticed people who work on an evergreen deployment can afford to work on a very granular, commit-level. However, if you have to support multiple production branches concurrently and often have to cherry-pick features and fixes across them, features will naturally become the basic unit of change you will find yourself gravitating towards, and will liberally use squash-merging just to keep your sanity.
Squash-merge is a scourge. I've seen squash merged commits 30 lines long ("try 15", empty line, "try 14", empty line...). I'm not even sure if you can do anything about such commits because squash-merge is a github/gitlab thing. So, I'm not sure if there are hooks to block it via a commit message linter.
And I've seen people going through some intense mental gymnastics to justify avoiding squashing locally, writing a proper commit message and then merging.
Agreed, it has been standard at most shops I've worked in the past 8-10 years.
This is too much thought put into a VCS. I don’t want to have to think about my VCS at all beyond the commit message. For all of Git’s popularity, I’ve never seen benefits that justify the absurd amount of work and knowledge it takes to perform simple actions. It’s the VCS equivalent of Scheme or emacs.
If you need to do a workaround, or a complicated feature sometime it's nice to explain it as a comment in the code, but sometime it's better to put it as a comment inside the commit message. But if it's all merged i the end, along with lots of template changes, README changes, refactor irrelevant to the current changes, then you're losing an important way of navigating a codebase.
Code tells how, not why—the domain of specs and comments. Commit messages effectively don’t exist for non-developers.
I put spec links in doc strings whenever possible. They are accessible to everyone—devs, PMs, SMEs, stakeholders that pay bills, and myself when at a web browser.
Searching is not required but even if it was it would be a tiny fraction of “hours.”
Fine as an opinion.
> For all of Git’s popularity, I’ve never seen benefits that justify the absurd amount of work and knowledge it takes to perform simple actions. It’s the VCS equivalent of Scheme or emacs.
This is just wrong. When people talk about this, it's not about git at all.
You write some code, and you make it into commits. Part of that is choosing if/how to organize it with multiple commits, and how much effort you want to put into that. This is fundamental to using a VCS, any VCS.
Or by analogy, if a lot of emacs users complain about your spelling, that's not because emacs is overly demanding.
It's like 4K porn. It's less appealing when you see everything.
The issue for me is when commit who are about adding a new value in the env file become mixed with template and responsive handling, mixed potentially with a bug fix.
Got fired. Kinda. I was laid off.
> 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.
Yes, any large organization has plenty of devs who all have their own style and preferences, for better or worse.
Whoever demands they all bend to the one true way is a fascist (lol not really but you know).
Just set up your CI/CD in such a way that PRs with weird git logs get squashed into one pretty message, preferably the PR description since other devs have to review the PR it's often given more effort. Set it up so that if things weren't formatted the "right way" they get auto-formatted or a test fails and the dev says "ah, I have to run that one task and then update the PR".
I don't think a big organization is going to scale with developer "evangelists" demanding people write their commits a certain way either.
conventionalcommits.org was the worst. I worked at one "big co" that tried to get devs to do this. Even after we had been doing it for a while, nobody ever went back to look at the history in such a way that it was worth it. We ended up throwing in the towel rather than the company trying to get all other teams to do it.
Ain't nobody got time for that shit.
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.
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.
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.
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.
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.
Do you really believe that if, for example, this change to btrfs filesystem https://lore.kernel.org/linux-btrfs/cover.1699470345.git.jos... would be squashed, nothing of value would be lost?
IMO, this is a lot simpler and easier to do than rebasing your branch to have a flawless history.
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.
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 -wA 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?
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.
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.
Right, but that's only because developers don't amend and force push their commits to the PR branch as they receive feedback. Which is largely encouraged by GitHub being a terrible code review tool.
To me, git is part of the development process, it's not an extra layer of friction on top. So I compose my commits as I go. I find it helpful for recording what I'm thinking as I write the code. If I wait till the very end, I'll have forgotten some important bit of context I wanted to include. So during the day I may use the commits like save points. But before I push anything I'll often check out a new branch and create and incremental set of commits that have the change broken down into digestible pieces. And if I receive feedback, I'll usually amend those changes into the PR and force push it.
I'd like to add that I spend a lot of time cleaning up tech debt. And I deal with a ton of commits and PRs that don't explain themselves. So I'm really biased toward a clean development workflow because I hope to make the lives of those who come after me easier.
I was also trained on this workflow by being an early git contributor and it had extremely high standards for documenting its work. There's a commit from Jeff King that's a one line change with about six paragraphs of explanation.
There's no right answer here. I value the "meta" part of writing code. Not everyone does and that's okay.
I have had bugfix cases where, digging through the repo history, both of those examples accidentally introduced the bug (the first because the person who made the original change didn't completely understand a business rule so it changed both the code and the test, the second because of a typo in python that only affected a small subset of the data). Keeping the commit separate let me see very quickly what happened and what the intent actually was.
EDIT: On top of that, there's usually a bit of 'related' work you need for a task, by example when you find an edge case related to your feature, and now you also needed to fix a bug, or you did a bit of refactoring on a related service, or needed to change the data on a badly formatted JSON file.
Unbeknownst to you, you added a bug when refactoring the related service, a bug that is spotted a few months after, only on a very specific edge case. If the cause is not obvious, you might want to reach for git bisect, but that won't be very useful now that everything I've talked about is squashed into a single commit.
I agree that's related work, but I'd argue that work doesn't belong in that branch. If you find a bug in the process of implementing a feature, create a bugfix branch that is merged separately. If you need to refactor a service, that's also a separate branch/PR.
That's actually the most common pushback I get from people when I talk about squashing. They say "but then a bunch of unrelated changes will be lumped together in the same commit", to which I respond, "why are a bunch of unrelated changes in the same branch/PR?"
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.
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.
That’s done by an automated tool. Correction of indentation is just a byproduct.
I don’t consider that “tampering”.
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.
I default my `git blame` to `git blame -w` which ignores whitespace commits. Though knowing how to jump back commits should be required knowledge.
$ git log --merges
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.
(I ask these questions fully assuming I'm doing it wrong.)
We aren’t talking about pushed code. We are talking about cleaning up the local commit history before pushing it into a shared branch.
Aside from that we need might need to clarify what the question is. With shared code & git, it’s nice to use a branch & merge workflow, and it’s nice to make incoming merges as clean / nice as you can do the resulting history is as smooth as it can be while capturing what happened at a reasonable granularity. These are today’s conventions though, and it’s really up to the team to decide how to balance shared work, and what people feel are the most important workflows and tools.
Aside from that, how are "a fix for a bug" style commits not "clean"? If merge 123 into master contains a bug that is fixed in a future merge 1234, it doesn't seem "dirty" to me; quite the opposite actually, as it tracks what actually happened.
Now, "wip" style commits shouldn't be on whatever main branch everyone is working on: that's what branches are for. And if everyone is just working off the main branch and committing directly to it, that's an organizational deficiency; not one that VCS can solve.
> “wip” style commits shouldn’t be on whatever branch everyone is working on
Agreed! We aren’t talking about rewriting shared branch history, we are talking about removing the “wip” commits made hastily and locally before pushing them. Sounds like we agree!
can you help me understand this? It is the exact opposite of my experience. The flow I see is: bug reported, write a git bisect test, identify the feature that introduced it, reach out to that developer/team.
This is allowed by squash merges. When I've seen these more "clean" histories, they have commit points that wont even compile or have runnable tests causing git bisect to fail.
> branches that are squash merged were big
it must be this - how big are your merges? All the projects I've worked on strive for smaller PRs. Large PRs are usually broken up into smaller pieces. Large PRs are an anti-pattern.
> how big are your merges? […] Large PRs are an anti-pattern.
Depends, but they sometimes on occasion can get pretty big, if there’s a bit refactor and/or multiple people in the branch. Small enough PRs are a nice goal - it’s a goal that might agree with and exist in part because squash merges on large PRs lose too much. It’s just the real world routinely gets in the way. It’s very easy for someone who needs to do an ‘atomic’ refactor to touch a ton of files. It’s very easy for a planned feature to end up way bigger than intended. You can’t always keep PRs small or enforce it on other people. Sometimes stuff happens, and when it does, sometimes squash merging feels less good than merging a branch with multiple commits. The good news is that it’s always optional. The bad news is that I can’t necessarily babysit or dictate what others do, and some people prefer squash-merging to spending any time doing cleanup on a messy branch.
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.
And for that matter, you'd manually do a squash with the interactive rebase tool anyway ("git rebase -i").
I call that a rebase.
I'll make a math analogy. Technically a rectangle is a trapezoid, but if someone says tries to draw a distinction between rectangles and proper trapezoids, it's not hard to figure out what they mean.
When rebase -i outputs a single commit, that's a degenerate case. There are statements about rebases that are generally true but not true for that specific kind.
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.