6 ms·

`git-bisect` is legit if you have to do the history archaeological digging. Though, there is the open question of how git commit history is maintained, the squa

by dpflan 11mo ago

`git-bisect` is legit if you have to do the history archaeological digging. Though, there is the open question of how git commit history is maintained, the squash-and-merge vs. just retain all history. With squash-and-merge you're looking at the merged pull-request versus with full history you can find the true code-level inflection point.

formerly_proven 11mo ago
> with full history you can find the true code-level inflection point. "typo fix"
- inopinatus 11mo ago
  “tues lunch wip”
echelon 11mo ago
Can someone explain why anyone would want non-squashed PRs? For the 5% of engineers that diligently split each PR into nice semantic changes, I suppose that's nice. But the vast majority of engineers don't do this. Individual commits in a PR are testing and iteration. You don't want to read though that. Unless, of course, you're asking the engineer to squash on their end before making the PR. But what's the value in that ceremony? Each PR being squashed to 1 commit is nice and easy to reason about. If you truly care about making more semantic history, split the work into multiple PRs. For that matter, why merge? Rebase it on top. It's so much cleaner. It's atomic and hermetic.
- stefan_ 11mo ago
  There is no free lunch, the same people that can't be bothered to make atomic semantic commits are the same people that will ruin your bisect with a commit that doesn't build or has some other unrelated run failure. People that don't care can't be fixed by tools. The advice around PRs rings hollow, after all they were invented by the very people that don't care - which is why they show all changes by default and hide the commits away, commit messages buried after 5 clicks. And because this profession is now filled with people that don't care, add the whole JIRA ticket and fix version rigmarole on top - all kinds of things that show up in some PMs report but not in my console fixing an issue that requires history.
- embedding-shape 11mo ago
  > For that matter, why merge? Rebase it on top. It's so much cleaner. It's atomic and hermetic. With an explicit merge, you keep two histories, yet mostly care about the "main" one. With rebase, you're effectively forgetting there ever was a separate history, and chose to rewrite the history when "effectively merging" (rebasing). There's value in both, mostly seems to come down to human preference. As long as the people that will be working with it agrees, I personally don't care either way which one, granted it's consistently applied.
  - eastbound 11mo ago
    Squashed PR allow you to see a single commit, while rebased PRs show up as multiple. The squash has the defect that you can’t rebase PRs that were on top. But a MERGE… is a single commit on Master, while keeping the detailed history! - We just don’t use the merge because they are ugly, - And they’re only ugly because the visualizers make them ugly. It’s a tooling problem. The merge is the correct implementation. (and yet I use the rebase-fast-forward).
    - thfuran 11mo ago
      You can squash after rebasing or merge a PR without squashing.
  - hinkley 11mo ago
    Rebasing is slower in practice to trunk based development and CI, while merging and squashing are moving farther away.
- nixpulvis 11mo ago
  I'd take fully squashed PRs over endless "fix thing" and "updated wip"... but if you work in a way that leaves a couple meaningful commits, that's even better. Sometimes I end up in this state naturally by having a feature branch, which I work on in sub branches, each being squashed into a single final commit. Or when the bulk of the logic is on one commit, but then a test case or two are added later, or a configuration needs changing. I like merge commits because they preserve the process of the review.
  - echelon 11mo ago
    > I like merge commits because they preserve the process of the review. I appreciate that, but I still can't square it with my world view. GitHub or whatever tool you use preserves all the comments and feedback and change history if you ever need to go back and reference it. Such instances are limited, and in my experience it's mainly politics and not technical when this happens. The team's PR discussion itself isn't captured in git, so it's lossy to expect this type of artifact to live in git anyway. It's also much less searchable and less first class than just going back to GitHub to access this. Ultimately, these software development artifacts aren't relevant to the production state of the software you're deploying. It feels muddled to put an incomplete version of it into your tree when the better source of truth lives outside.
    - nixpulvis 11mo ago
      Usually the merge commit is what has the link to the PR/MR. So it's the best way to actually find it. > Ultimately, these software development artifacts aren't relevant to the production state of the software you're deploying. It feels muddled to put an incomplete version of it into your tree when the better source of truth lives outside. You could make the same claim about the entire history. Git is a development tool, production just needs a working cut of the source.
  - hinkley 11mo ago
    What you have there is people hiding their neuroses and lack of commit hygiene and that’s avoiding the problem not fixing it.
- nothrabannosir 11mo ago
  Because github doesn't support stacked diffs, basically. T_T
- rectang 11mo ago
  Crafting a PR as an easily-consumed, logical sequence of commits is particularly useful in open source. 1. It makes review much easier, which is both important because core maintainer effort is the most precious resource in open source, and because it increases the likelihood that your PR will be accepted. 2. It makes it easier for people to use the history for analysis, which is especially important when you may not be able to speak directly to the original author. These reasons also apply in commercial environments of course, but to a lesser extent. For me, organizing my PRs this way is second nature and only nominal effort, because I'm extremely comfortable with Git, including the following idiom which serves as a more powerful form of `git commit --amend`: git add -p git commit --fixup COMMIT_ID git stash git rebase -i --autosquash COMMIT_ID~ An additional benefit is that this methodology doesn't work well for huge changesets, so it discourages the anti-pattern of long-lived topic branches. :) > For that matter, why merge? Rebase it on top. Yes, that works for me although it might not work for people who aren't going to the same lengths to craft a logical history. I have no interest in preserving my original WIP commits — my goal is to create something that is easy to review. BUT... the PR should ultimately be merged with a merge commit. Then when you have a bug you can run `git bisect` on merges only, which is good enough.
  - vjerancrnjak 11mo ago
    I have an exactly opposite preference. Give me a big change to review. Going commit by commit or any imposed steps is not how I write code or how I understand code. If you did not approach it through literate programming, I just prefer all of the thousands of lines at once.
    - rectang 11mo ago
      You're in luck! For you, there is `git diff`. (And for me as well — both the individual commits and the PR-level summary are useful.) So, those of us who prefer commit-sized chunking don't have to do anything special to accommodate your preference. It doesn't go the other way, of course, if you present one big commit to me. But so long as your code is well-commented (heh) and the PR isn't too huge (heh heh) and you don't intersperse file renamings (heh heh heh) or code formatting changes (heh heh heh heh) which make it extremely difficult to see what you changed... no problem!
- mikeocool 11mo ago
  If you ever worked with stacked PRs, and the top one gets squashed and merged it often becomes a nightmare to rebase the rest of the PRs to bring them up to date.
  - rectang 11mo ago
    I wish this was easier. I have a workflow that I use to create stacked PRs which involves changing the target branch of the next PR to `main` after merging its predecessor, but it is too fragile to institute as a policy. However, this is also just a more specific version of the general problem that long-lived, elaborate topic branches are difficult to work with.
  - baq 11mo ago
    jj makes this mostly trivial - well worth checking out if you’d like to work this way, but GitHub gets in the way.
  - sebastianmestre 11mo ago
    git rebase --onto squashed nonsquashed next-pr Works fine in my experience (except when submodules are involved)
- leptons 11mo ago
  I manage a team of developers, and I don't think any of us squash commits, and I really don't care. It's been working fine for 8 years at this job. We keep our git use extremely simple, we don't spend much time even thinking about git. The most we do with git is commit, push, and merge (and stash is useful too). Never need to rebase or get any deeper into git. Doing anything complicated with git is wasting development time. Squashing commits isn't useful to us at all. We have too much forward velocity to worry that some commit isn't squashed. If a bug does come up, we move forward and fix it, the git history doesn't really figure into our process much, if at all.
  - ziml77 11mo ago
    Squash is just something you do at the point of merging. It's a single option during the merge and doesn't have you doing any more work than a merge that you don't squash. I don't know about github, but I know in gitlab it's a simple checkbox in the merge request (and it can be set to be checked by default by the admin if they want).
    - leptons 11mo ago
      That's great for you, but squashing commits doesn't do anything for our team. It hides somewhat useful information, which never seemed like a good thing to me. Other teams at my company are obsessive about squashing, and I am glad I don't work on those teams. They are frustrating to work with, they are notoriously slow to ship anything, and their product still breaks even with all their processes and hurdles to getting anything shipped. Teams that demand squashing simply don't impress me at all.
- SatvikBeri 11mo ago
  Making human-readable commit history is not that hard with a little practice. It's one of the big benefits of tools like magit or jj. My team started doing it a few weeks ago, and it's made reviewing PRs substantially easier.
  - criemen 11mo ago
    +1: EdaMagit has been a game changer for me wrt reordering commits, fusing them together, and writing at least 1-2 sentences of proper commit messages after the fact.
- CogitoCogito 11mo ago
  I have always been very careful with git histories and often rewrite/squash them before final review/merge. Often my rewritten histories have nothing to do with the original history and commits are logically/intuitively separated and individually testable. That said, very few people seem to be like me. Most people have no concept of what a clear commit history is. I think it's kind of similar to how most people are terrible written communicators. Few people have any clue how to express themselves clearly. The easiest way to deal with people like this is to just have them squash their PRs. This way you can at least enforce some sanity at review and then the final commit should enforce some standards. I agree on rebasing instead of straight merging, but even that's too complicated for most people.
- bentcorner 11mo ago
  > If you truly care about making more semantic history, split the work into multiple PRs. This exactly - if your commit history for a PR is interesting enough to split apart, then the original PR was too large and should have been split up to begin with. This is also a team culture thing - people won't make "clean" commits into a PR if they know people aren't going to be bisecting into them and trying to build. OTOH, having people spend time prepping good commits is potentially time wasted if nobody ever looks at the PR commit history aside from the PR reviewers.
  - hamburglar 11mo ago
    If I have a feature branch, and as part of that feature change, I did a simple refactor of something, I definitely want that committed as two separate commits in the PR, because you can look at the changes in isolation and it makes them a LOT easier to follow. And if you prefer, you can look at the diff of the entire PR in one single view. I don’t see the downside. And please do not come back with “you shouldn’t do a refactor as part of a feature change.” We don’t need to add bureaucracy to avoid a problem caused by failure to understand the power of good version control.
    - normie3000 11mo ago
      This bureaucracy has very low overhead. Squash-merge the feature and then the refactor, or the refactor then the feature. Also makes reviewing each quicker.
      - PaulDavisThe1st 11mo ago
        the feature isn't ready for merge when the refactor happens ....
      - hamburglar 11mo ago
        Requiring me to do those changes in series as separate merges is the bureaucracy. There is a reason pull requests and merges operate on branches, not just individual commits. It’s like you’re intentionally hamstringing yourself by saying these things must appear as a single commit. Do I want every PR to be a long ugly list of trivial half-done commits with messages like “fix typo” or “partial checkpoint of encabulator functionality”? No. Does everything need to be hammered down into a single wad? Also no. There is a middle ground and people should be trusted to find it, and even to have their own opinion on where it is.
- koolba 11mo ago
  > Can someone explain why anyone would want non-squashed PRs? So you can differentiate the plumbing from the porcelain. If all the helpers, function expansions, typo corrections, and general renamings are isolated, what remains is the pure additional functional changes on its own. It makes reviewing changes much easier.
- vjerancrnjak 11mo ago
  You can just inspect merge commits, you can also just bisect over merge commits. Splitting work into multiple PRs is unnecessary ritual. I have never reasoned about git history or paid attention to most commit messages or found any of it useful compared to the contents of the change. When I used git bisect with success it was on unknown projects. Known projects are easy to debug.
- pizza234 11mo ago
  > Each PR being squashed to 1 commit is nice and easy to reason about. If you truly care about making more semantic history, split the work into multiple PRs. I don't argue with your point (even if I am obsessive about commits separation), but one needs to keep in mind that the reverse also applies, that is, on other end of the spectrum, there are devs who create kitchen-sink PRs which include, for example, refactorings, which make squashed PRs harder to reasons about.
- borntyping 11mo ago
  > Can someone explain why anyone would want non-squashed PRs? > > For the 5% of engineers that diligently split each PR into nice semantic changes, I suppose that's nice. But the vast majority of engineers don't do this. I think cause and effect are the other way around here. You write and keep work-in-progress commits without caring about changes because the history will be discarded and the team will only look at pull requests as a single unit, and write tidy distinct commits because the history will be kept and individual commits will be reviewed. I've done both, and getting everyone to do commits properly is much nicer, though GitHub and similar tools don't really support or encourage it. If you work with repository history a lot (for example, you have important repositories that aren't frequently committed to, or maintain many different versions of the project) it's invaluable. Most projects don't really care about the history—only the latest changes—and work with pull-requests, which is why they tend to use the squashed pull request approach.
  - baq 11mo ago
    It’s mostly because pull requests are what is being tested in CI, not individual commits. Might as well squash as nobody wants to deal with untested in-between crap. If you mean stacked PRs, yeah GitHub absolutely sucks. Gerrit a decade ago was a better experience.
    - adastra22 11mo ago
      That's a problem with your CI. You can configure it to test all commits which make up the PR.
      - baq 11mo ago
        My CI takes 20+ mins and costs meaningful $ per run. Not happening.
        adastra22 11mo ago
        > That's a problem with your CI.
- adastra22 11mo ago
  > For the 5% of engineers that diligently split each PR into nice semantic changes, I suppose that's nice. But the vast majority of engineers don't do this. Here's a simple reason: at my company, if you don't do this, you get fired. This is basic engineering hygiene.
  - devnullbrain 11mo ago
    Yep. Those 5% shouldn't be approving PRs without it. Those 95% shouldn't be code owners. Nice things can be had, you just have to work for them.
- mkleczek 11mo ago
  git merge --no-ff git log --first-parent git bisect --first-parent The above gives you clean PR history in the main branch while retaining detailed work history in (merged) feature branches. I really don't understand why would I squash having git merge --no-ff at my disposal...
  - rectang 11mo ago
    +1, I have `git merge --no-ff` and `git pull --ff-only` set in my global gitconfig[1]. [1] https://github.com/rectang/dotfiles/blob/master/gitextra#L11C1-L14C11 https://github.com/rectang/dotfiles/blob/master/gitextra#L11...
- anonymars 11mo ago
  Hoo boy is it fun to figure out where things went wrong when the real commit history was thrown away to make it look prettier. Especially a mistake from a merge conflict.
  - hinkley 11mo ago
    I had to stop two devs from fisticuffs. One of them used merge and manufactured a bug in the other’s code and was making a stink about it in the middle of the cubicles. Merges lie I worse ways than rebase does. Hands down. With rebase I break my own shit. With merge I can break yours. Since your code is already merges into trunk, it has fewer eyes on it now and it’s on me to make sure my code works with yours and not vice versa.
    - anonymars 11mo ago
      I don't follow. In either case two branches are combined into one: With a merge commit anyone can see each original path, and the merge result (with its author), and even try the three-way merge again. With a rebase, the original context is lost (the original commits were replaced by the rebased versions). A rebase is a lossy projection of the same operation. A rebase lies about its history, a merge does not.
      - hinkley 11mo ago
        Merge with conflicts can be resolved with changes in code that don’t belong to the person who resolved the merge, and are not attributed to the person who resolved the merge. Git blame shows someone else as the person who introduced the bug. When you do a rebase you’re only modifying your own commits, or those of someone sharing your branch.
        anonymars 11mo ago
        I'd really like to see an example of what you mean. Are you saying a git blame is attributing code to a person who did not write that code? Are cherry picks involved (which also screw up history)? https://devblogs.microsoft.com/oldnewthing/20180313-00/?p=98225 https://devblogs.microsoft.com/oldnewthing/20180313-00/?p=98...
- lucasoshiro 11mo ago
  I wrote a text about that: https://lucasoshiro.github.io/posts-en/2024-04-08-please_dont_squash/ https://lucasoshiro.github.io/posts-en/2024-04-08-please_don...
- kragen 11mo ago
  I would definitely neither accept a pull request where the individual commits were testing and iteration, nor a pull request with hundreds of lines of changes are in a single commit. (New code, maybe.) It's not about ceremony; it's about knowing what changed and why.
- WorldMaker 11mo ago
  Why would you want to restrict yourself to a one-dimensional mentality of commits when the source control system handles two-dimensions naturally and well? It seems to me a bit like wanting to only use about half the tools that you have available. Squashes and Rebases delete information entirely out of source control. If a PR adds 100 lines of code, then deletes 50 of them and you squash it, you've lost 50 lines of code. Once your PR tool garbage collects those original commits you can't easily get it back. Maybe that's fine and that was garbage code anyway, but it was part of the process of getting those 50 lines you kept and could be relevant to questions of "why were these 50 lines added?" Were those other 50 lines alternatives that were rejected in testing, perhaps? Worse, were they deleted on accident in a bad inner merge and not caught in time? I feel a lot less confident in the YAGNI code tossing that Squashes and Rebases do. I've had too many times where I've needed to code dive a fix out of a Merge commit or out of a long winding PR that went through several alternatives. Git provides some great 2D navigation tools such as `--first-parent`. You can have "I just want a view of PR merges" and the structure of "the commits below that PR are accessible, too". I can't see why you wouldn't want that. That's what source control is for, "save all the minutia in case I need to find it again someday" and it is just a UI problem if you don't want the 2D view by default. I think the "subway diagrams" that most git UIs show look pretty, especially in screenshots, but are not the UI most people actually want to work with day-to-day. I think a lot fewer people would be so adamant about squash-only or rebase-only workflows if more git UIs took a "1D" first view of the git DAG instead of "subway views". I think more git UIs should start with `--first-parent` and provide drilldown into "lower" commits instead.
- chrishill89 11mo ago
  The question as stated answers itself. Some are able to make nice multi-commit PRs. So they do.
WorldMaker 11mo ago
`git bisect --first-parent` lets you easily restrict to just the "PR level" if you are using PRs with merge commits (ie, no-fast-forward).

by dpflan 11mo ago

formerly_proven 11mo ago

> with full history you can find the true code-level inflection point. "typo fix"

inopinatus 11mo ago
“tues lunch wip”

echelon 11mo ago

Can someone explain why anyone would want non-squashed PRs? For the 5% of engineers that diligently split each PR into nice semantic changes, I suppose that's nice. But the vast majority of engineers don't do this. Individual commits in a PR are testing and iteration. You don't want to read though that. Unless, of course, you're asking the engineer to squash on their end before making the PR. But what's the value in that ceremony? Each PR being squashed to 1 commit is nice and easy to reason about. If you truly care about making more semantic history, split the work into multiple PRs. For that matter, why merge? Rebase it on top. It's so much cleaner. It's atomic and hermetic.

stefan_ 11mo ago
There is no free lunch, the same people that can't be bothered to make atomic semantic commits are the same people that will ruin your bisect with a commit that doesn't build or has some other unrelated run failure. People that don't care can't be fixed by tools. The advice around PRs rings hollow, after all they were invented by the very people that don't care - which is why they show all changes by default and hide the commits away, commit messages buried after 5 clicks. And because this profession is now filled with people that don't care, add the whole JIRA ticket and fix version rigmarole on top - all kinds of things that show up in some PMs report but not in my console fixing an issue that requires history.
embedding-shape 11mo ago
> For that matter, why merge? Rebase it on top. It's so much cleaner. It's atomic and hermetic. With an explicit merge, you keep two histories, yet mostly care about the "main" one. With rebase, you're effectively forgetting there ever was a separate history, and chose to rewrite the history when "effectively merging" (rebasing). There's value in both, mostly seems to come down to human preference. As long as the people that will be working with it agrees, I personally don't care either way which one, granted it's consistently applied.
- eastbound 11mo ago
  Squashed PR allow you to see a single commit, while rebased PRs show up as multiple. The squash has the defect that you can’t rebase PRs that were on top. But a MERGE… is a single commit on Master, while keeping the detailed history! - We just don’t use the merge because they are ugly, - And they’re only ugly because the visualizers make them ugly. It’s a tooling problem. The merge is the correct implementation. (and yet I use the rebase-fast-forward).
  - thfuran 11mo ago
    You can squash after rebasing or merge a PR without squashing.
- hinkley 11mo ago
  Rebasing is slower in practice to trunk based development and CI, while merging and squashing are moving farther away.
nixpulvis 11mo ago
I'd take fully squashed PRs over endless "fix thing" and "updated wip"... but if you work in a way that leaves a couple meaningful commits, that's even better. Sometimes I end up in this state naturally by having a feature branch, which I work on in sub branches, each being squashed into a single final commit. Or when the bulk of the logic is on one commit, but then a test case or two are added later, or a configuration needs changing. I like merge commits because they preserve the process of the review.
- echelon 11mo ago
  > I like merge commits because they preserve the process of the review. I appreciate that, but I still can't square it with my world view. GitHub or whatever tool you use preserves all the comments and feedback and change history if you ever need to go back and reference it. Such instances are limited, and in my experience it's mainly politics and not technical when this happens. The team's PR discussion itself isn't captured in git, so it's lossy to expect this type of artifact to live in git anyway. It's also much less searchable and less first class than just going back to GitHub to access this. Ultimately, these software development artifacts aren't relevant to the production state of the software you're deploying. It feels muddled to put an incomplete version of it into your tree when the better source of truth lives outside.
  - nixpulvis 11mo ago
    Usually the merge commit is what has the link to the PR/MR. So it's the best way to actually find it. > Ultimately, these software development artifacts aren't relevant to the production state of the software you're deploying. It feels muddled to put an incomplete version of it into your tree when the better source of truth lives outside. You could make the same claim about the entire history. Git is a development tool, production just needs a working cut of the source.
- hinkley 11mo ago
  What you have there is people hiding their neuroses and lack of commit hygiene and that’s avoiding the problem not fixing it.
nothrabannosir 11mo ago
Because github doesn't support stacked diffs, basically. T_T
rectang 11mo ago
Crafting a PR as an easily-consumed, logical sequence of commits is particularly useful in open source. 1. It makes review much easier, which is both important because core maintainer effort is the most precious resource in open source, and because it increases the likelihood that your PR will be accepted. 2. It makes it easier for people to use the history for analysis, which is especially important when you may not be able to speak directly to the original author. These reasons also apply in commercial environments of course, but to a lesser extent. For me, organizing my PRs this way is second nature and only nominal effort, because I'm extremely comfortable with Git, including the following idiom which serves as a more powerful form of `git commit --amend`: git add -p git commit --fixup COMMIT_ID git stash git rebase -i --autosquash COMMIT_ID~ An additional benefit is that this methodology doesn't work well for huge changesets, so it discourages the anti-pattern of long-lived topic branches. :) > For that matter, why merge? Rebase it on top. Yes, that works for me although it might not work for people who aren't going to the same lengths to craft a logical history. I have no interest in preserving my original WIP commits — my goal is to create something that is easy to review. BUT... the PR should ultimately be merged with a merge commit. Then when you have a bug you can run `git bisect` on merges only, which is good enough.
- vjerancrnjak 11mo ago
  I have an exactly opposite preference. Give me a big change to review. Going commit by commit or any imposed steps is not how I write code or how I understand code. If you did not approach it through literate programming, I just prefer all of the thousands of lines at once.
  - rectang 11mo ago
    You're in luck! For you, there is `git diff`. (And for me as well — both the individual commits and the PR-level summary are useful.) So, those of us who prefer commit-sized chunking don't have to do anything special to accommodate your preference. It doesn't go the other way, of course, if you present one big commit to me. But so long as your code is well-commented (heh) and the PR isn't too huge (heh heh) and you don't intersperse file renamings (heh heh heh) or code formatting changes (heh heh heh heh) which make it extremely difficult to see what you changed... no problem!
mikeocool 11mo ago
If you ever worked with stacked PRs, and the top one gets squashed and merged it often becomes a nightmare to rebase the rest of the PRs to bring them up to date.
- rectang 11mo ago
  I wish this was easier. I have a workflow that I use to create stacked PRs which involves changing the target branch of the next PR to `main` after merging its predecessor, but it is too fragile to institute as a policy. However, this is also just a more specific version of the general problem that long-lived, elaborate topic branches are difficult to work with.
- baq 11mo ago
  jj makes this mostly trivial - well worth checking out if you’d like to work this way, but GitHub gets in the way.
- sebastianmestre 11mo ago
  git rebase --onto squashed nonsquashed next-pr Works fine in my experience (except when submodules are involved)
leptons 11mo ago
I manage a team of developers, and I don't think any of us squash commits, and I really don't care. It's been working fine for 8 years at this job. We keep our git use extremely simple, we don't spend much time even thinking about git. The most we do with git is commit, push, and merge (and stash is useful too). Never need to rebase or get any deeper into git. Doing anything complicated with git is wasting development time. Squashing commits isn't useful to us at all. We have too much forward velocity to worry that some commit isn't squashed. If a bug does come up, we move forward and fix it, the git history doesn't really figure into our process much, if at all.
- ziml77 11mo ago
  Squash is just something you do at the point of merging. It's a single option during the merge and doesn't have you doing any more work than a merge that you don't squash. I don't know about github, but I know in gitlab it's a simple checkbox in the merge request (and it can be set to be checked by default by the admin if they want).
  - leptons 11mo ago
    That's great for you, but squashing commits doesn't do anything for our team. It hides somewhat useful information, which never seemed like a good thing to me. Other teams at my company are obsessive about squashing, and I am glad I don't work on those teams. They are frustrating to work with, they are notoriously slow to ship anything, and their product still breaks even with all their processes and hurdles to getting anything shipped. Teams that demand squashing simply don't impress me at all.
SatvikBeri 11mo ago
Making human-readable commit history is not that hard with a little practice. It's one of the big benefits of tools like magit or jj. My team started doing it a few weeks ago, and it's made reviewing PRs substantially easier.
- criemen 11mo ago
  +1: EdaMagit has been a game changer for me wrt reordering commits, fusing them together, and writing at least 1-2 sentences of proper commit messages after the fact.
CogitoCogito 11mo ago
I have always been very careful with git histories and often rewrite/squash them before final review/merge. Often my rewritten histories have nothing to do with the original history and commits are logically/intuitively separated and individually testable. That said, very few people seem to be like me. Most people have no concept of what a clear commit history is. I think it's kind of similar to how most people are terrible written communicators. Few people have any clue how to express themselves clearly. The easiest way to deal with people like this is to just have them squash their PRs. This way you can at least enforce some sanity at review and then the final commit should enforce some standards. I agree on rebasing instead of straight merging, but even that's too complicated for most people.
bentcorner 11mo ago
> If you truly care about making more semantic history, split the work into multiple PRs. This exactly - if your commit history for a PR is interesting enough to split apart, then the original PR was too large and should have been split up to begin with. This is also a team culture thing - people won't make "clean" commits into a PR if they know people aren't going to be bisecting into them and trying to build. OTOH, having people spend time prepping good commits is potentially time wasted if nobody ever looks at the PR commit history aside from the PR reviewers.
- hamburglar 11mo ago
  If I have a feature branch, and as part of that feature change, I did a simple refactor of something, I definitely want that committed as two separate commits in the PR, because you can look at the changes in isolation and it makes them a LOT easier to follow. And if you prefer, you can look at the diff of the entire PR in one single view. I don’t see the downside. And please do not come back with “you shouldn’t do a refactor as part of a feature change.” We don’t need to add bureaucracy to avoid a problem caused by failure to understand the power of good version control.
  - normie3000 11mo ago
    This bureaucracy has very low overhead. Squash-merge the feature and then the refactor, or the refactor then the feature. Also makes reviewing each quicker.
    - PaulDavisThe1st 11mo ago
      the feature isn't ready for merge when the refactor happens ....
    - hamburglar 11mo ago
      Requiring me to do those changes in series as separate merges is the bureaucracy. There is a reason pull requests and merges operate on branches, not just individual commits. It’s like you’re intentionally hamstringing yourself by saying these things must appear as a single commit. Do I want every PR to be a long ugly list of trivial half-done commits with messages like “fix typo” or “partial checkpoint of encabulator functionality”? No. Does everything need to be hammered down into a single wad? Also no. There is a middle ground and people should be trusted to find it, and even to have their own opinion on where it is.
koolba 11mo ago
> Can someone explain why anyone would want non-squashed PRs? So you can differentiate the plumbing from the porcelain. If all the helpers, function expansions, typo corrections, and general renamings are isolated, what remains is the pure additional functional changes on its own. It makes reviewing changes much easier.
vjerancrnjak 11mo ago
You can just inspect merge commits, you can also just bisect over merge commits. Splitting work into multiple PRs is unnecessary ritual. I have never reasoned about git history or paid attention to most commit messages or found any of it useful compared to the contents of the change. When I used git bisect with success it was on unknown projects. Known projects are easy to debug.
pizza234 11mo ago
> Each PR being squashed to 1 commit is nice and easy to reason about. If you truly care about making more semantic history, split the work into multiple PRs. I don't argue with your point (even if I am obsessive about commits separation), but one needs to keep in mind that the reverse also applies, that is, on other end of the spectrum, there are devs who create kitchen-sink PRs which include, for example, refactorings, which make squashed PRs harder to reasons about.
borntyping 11mo ago
> Can someone explain why anyone would want non-squashed PRs? > > For the 5% of engineers that diligently split each PR into nice semantic changes, I suppose that's nice. But the vast majority of engineers don't do this. I think cause and effect are the other way around here. You write and keep work-in-progress commits without caring about changes because the history will be discarded and the team will only look at pull requests as a single unit, and write tidy distinct commits because the history will be kept and individual commits will be reviewed. I've done both, and getting everyone to do commits properly is much nicer, though GitHub and similar tools don't really support or encourage it. If you work with repository history a lot (for example, you have important repositories that aren't frequently committed to, or maintain many different versions of the project) it's invaluable. Most projects don't really care about the history—only the latest changes—and work with pull-requests, which is why they tend to use the squashed pull request approach.
- baq 11mo ago
  It’s mostly because pull requests are what is being tested in CI, not individual commits. Might as well squash as nobody wants to deal with untested in-between crap. If you mean stacked PRs, yeah GitHub absolutely sucks. Gerrit a decade ago was a better experience.
  - adastra22 11mo ago
    That's a problem with your CI. You can configure it to test all commits which make up the PR.
    - baq 11mo ago
      My CI takes 20+ mins and costs meaningful $ per run. Not happening.
      - adastra22 11mo ago
        > That's a problem with your CI.
adastra22 11mo ago
> For the 5% of engineers that diligently split each PR into nice semantic changes, I suppose that's nice. But the vast majority of engineers don't do this. Here's a simple reason: at my company, if you don't do this, you get fired. This is basic engineering hygiene.
- devnullbrain 11mo ago
  Yep. Those 5% shouldn't be approving PRs without it. Those 95% shouldn't be code owners. Nice things can be had, you just have to work for them.
mkleczek 11mo ago
git merge --no-ff git log --first-parent git bisect --first-parent The above gives you clean PR history in the main branch while retaining detailed work history in (merged) feature branches. I really don't understand why would I squash having git merge --no-ff at my disposal...
- rectang 11mo ago
  +1, I have `git merge --no-ff` and `git pull --ff-only` set in my global gitconfig[1]. [1] https://github.com/rectang/dotfiles/blob/master/gitextra#L11C1-L14C11 https://github.com/rectang/dotfiles/blob/master/gitextra#L11...
anonymars 11mo ago
Hoo boy is it fun to figure out where things went wrong when the real commit history was thrown away to make it look prettier. Especially a mistake from a merge conflict.
- hinkley 11mo ago
  I had to stop two devs from fisticuffs. One of them used merge and manufactured a bug in the other’s code and was making a stink about it in the middle of the cubicles. Merges lie I worse ways than rebase does. Hands down. With rebase I break my own shit. With merge I can break yours. Since your code is already merges into trunk, it has fewer eyes on it now and it’s on me to make sure my code works with yours and not vice versa.
  - anonymars 11mo ago
    I don't follow. In either case two branches are combined into one: With a merge commit anyone can see each original path, and the merge result (with its author), and even try the three-way merge again. With a rebase, the original context is lost (the original commits were replaced by the rebased versions). A rebase is a lossy projection of the same operation. A rebase lies about its history, a merge does not.
    - hinkley 11mo ago
      Merge with conflicts can be resolved with changes in code that don’t belong to the person who resolved the merge, and are not attributed to the person who resolved the merge. Git blame shows someone else as the person who introduced the bug. When you do a rebase you’re only modifying your own commits, or those of someone sharing your branch.
      - anonymars 11mo ago
        I'd really like to see an example of what you mean. Are you saying a git blame is attributing code to a person who did not write that code? Are cherry picks involved (which also screw up history)? https://devblogs.microsoft.com/oldnewthing/20180313-00/?p=98225 https://devblogs.microsoft.com/oldnewthing/20180313-00/?p=98...
lucasoshiro 11mo ago
I wrote a text about that: https://lucasoshiro.github.io/posts-en/2024-04-08-please_dont_squash/ https://lucasoshiro.github.io/posts-en/2024-04-08-please_don...
kragen 11mo ago
I would definitely neither accept a pull request where the individual commits were testing and iteration, nor a pull request with hundreds of lines of changes are in a single commit. (New code, maybe.) It's not about ceremony; it's about knowing what changed and why.
WorldMaker 11mo ago
Why would you want to restrict yourself to a one-dimensional mentality of commits when the source control system handles two-dimensions naturally and well? It seems to me a bit like wanting to only use about half the tools that you have available. Squashes and Rebases delete information entirely out of source control. If a PR adds 100 lines of code, then deletes 50 of them and you squash it, you've lost 50 lines of code. Once your PR tool garbage collects those original commits you can't easily get it back. Maybe that's fine and that was garbage code anyway, but it was part of the process of getting those 50 lines you kept and could be relevant to questions of "why were these 50 lines added?" Were those other 50 lines alternatives that were rejected in testing, perhaps? Worse, were they deleted on accident in a bad inner merge and not caught in time? I feel a lot less confident in the YAGNI code tossing that Squashes and Rebases do. I've had too many times where I've needed to code dive a fix out of a Merge commit or out of a long winding PR that went through several alternatives. Git provides some great 2D navigation tools such as `--first-parent`. You can have "I just want a view of PR merges" and the structure of "the commits below that PR are accessible, too". I can't see why you wouldn't want that. That's what source control is for, "save all the minutia in case I need to find it again someday" and it is just a UI problem if you don't want the 2D view by default. I think the "subway diagrams" that most git UIs show look pretty, especially in screenshots, but are not the UI most people actually want to work with day-to-day. I think a lot fewer people would be so adamant about squash-only or rebase-only workflows if more git UIs took a "1D" first view of the git DAG instead of "subway views". I think more git UIs should start with `--first-parent` and provide drilldown into "lower" commits instead.
chrishill89 11mo ago
The question as stated answers itself. Some are able to make nice multi-commit PRs. So they do.

WorldMaker 11mo ago

`git bisect --first-parent` lets you easily restrict to just the "PR level" if you are using PRs with merge commits (ie, no-fast-forward).