13 ms·

I will always fight tooth and nail against squash merge. Squash merge has the major disadvantage of getting rid of valuable meaningful git history. Squash mer

I will always fight tooth and nail against squash merge. Squash merge has the major disadvantage of getting rid of valuable meaningful git history. Squash merge is not the proper solution for keeping your git history clean, it is a hack using the side effect of squash. Keeping your git history clean is a matter of policy, best-practices and education: Developers should be required to submit _clean_ PRs, that is, PR's whose git history has been organized and refactored in such a way that it removed "clean up commits", "typo fix", etc. When you squash merge a feature branch that has thousands of lines of code, and 6 months later you have a bug introduced by this feature branch, it becomes extremely hard to find which line introduced the bug. On the other hand, if you kept the history, and if this history was clean from the get go, it becomes easy to read the commits one-by-one and understand the issue. Don't use squash+merge. edit: I see numerous comment saying, in essence, "squash+merge" is what gets rid of the dirty history. No, developers must learn the existence of `git rebase --interactive`, which is the command to use to clean your git history[0]. "squash" is one possible action, among others, that helps cleaning the history. [0]: https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History

calpaterson 5y ago
> Developers should be required to submit _clean_ PRs, that is, PR's whose git history has been organized and refactored in such a way that it removed "clean up commits", "typo fix", etc. A complete and utter waste of time. You spend more time messing about with rebase than solving problems. When you're digging through VCS history due to a bug you often ignore the commit message anyway - if the code did what it seemed to do you wouldn't be there.
- preordained 5y ago
  Could not agree more...I wish I had more of substance to add, but an upvote didn't feel sufficient
- qudat 5y ago
  Agreed but I also think this depends heavily on context. If the organization cares a lot of clean commit messages, have a nice git history, and want PRs to be submitted in a clean state, great! This is not always the case. I've worked on codebases where we rarely scan the git history messages for anything useful, it's just as easy for us to dig into the code to figure out what's wrong.
- dyeje 5y ago
  It's a prime example of engineers mistaking aesthetics for utility.
- tablespoon 5y ago
  > A complete and utter waste of time. You spend more time messing about with rebase than solving problems. Eh. Not really. A clean history is a good resource for figuring out what was changed and why. Cleaning up history isn't even that much work anyway. > When you're digging through VCS history due to a bug you often ignore the commit message anyway - if the code did what it seemed to do you wouldn't be there. Only if the commit messages are shitty, which unfortunately they often are. I try to summarize what I'm changing and what I indent those changes to accomplish, because if there's one thing I hate doing, it's spending more time than necessary to puzzle out what I (or a coworker) was thinking at the time. VCS history can be a pretty powerful tool, but a messy history discourages the use of it.
  - hbogert 5y ago
    amen! really, do people even use git bisect? If you do, you go through some rite of passage and I've seen people including myself make better (not perfect) commits. It remains a black art to gauge what the perfect amount of work in a commit should be.
- throwaway1777 5y ago
  Strongly disagree. Mainline branch commit history should be clean atomic commits that you can roll back. Otherwise you are just asking for trouble on any merge conflict or hotfix scenario.
- pizza234 5y ago
  > A complete and utter waste of time. You spend more time messing about with rebase than solving problems. I'm also one of those "micro committers". Because of this attitude, many hundreds pedantic commit rebases after, I'm now a better programmer (and I also spend next to no time with that type of rebases). This is because I have now a much higher capacity (and speed) of breaking down problems into smaller, self-contained, steps. There is a common understanding that VCs are just storages. VCs actually map the mental model of the developer. Having a precise, clear, granular VC history has a bidirectional relationship with being a precise, granular, clear developer.
- mrkeen 5y ago
  > You spend more time messing about with rebase than solving problems Rebase should be quick and straightforward. If it's not, you've got bigger problems. By rebasing, you're taking responsibility for changing the codebase as it exists now, rather than how it used to be.
- sebastialonso 5y ago
  I think I'm missing something here. How valuable is to have 20 commits of "fix this error" , "fix the fix of the error", "revert all fixes", "real fix".... etc? I'd argue a PR with many commits such as these, conveys little no no useful information, when the actual change is 1-3 LOC. what about cleaning and filtering out useless commits, by soft reseting the branch and commiting just the actual changes to merge. Sure, it can take a lot of time, but it's done once, by you, after all you're supplying the changes. And if it takes a really long time, you're doing it wrong in the first place by submitting PRs with many LOC. Have you tried rebasing branches with dozens of short useless commits? That's a real waste of time, and everyone needs to do this if you want to update your main branch. So you've now multiplied the amount of wasted time for everyone.
  - darknavi 5y ago
    > what about cleaning and filtering out useless commits, by soft reseting the branch and commiting just the actual changes to merge. So... squashing?
    - phone8675309 5y ago
      Yes, but the author of the code, who would presumably know their intent, gets to choose how many commits to squash to instead of just one. This means that they can preserve the development process in a logical way.
    - u801e 5y ago
      What if the change really should be split into multiple commits. Having a mega commit affecting hundreds of lines across tens of files doesn't make reviewing the change easy, and reverting it will result in conflicts.
      - Seb-C 5y ago
        I would ask such changes to be split into multiple independent PRs
- thom 5y ago
  Do people here have examples of some bugs for which they had to resort to VCS history to find the cause? I'm struggling to picture a single bug in my whole career where this would have been quicker than just following the logic of the code. If there's information in commit messages that isn't evident in the code itself, that seems a terrible way to live.
  - pizza234 5y ago
    As wrote in another comment, bisecting (which is for me a significant tool) relies on history (specifically, a granular one). However, it also must a disciplined history.
    - thom 5y ago
      What's an example of a bug you've had to bisect for recently? Forgive me, it just seems like such a last resort thing.
      - LocalPCGuy 5y ago
        I don't bisect regularly, but when I do, it's usually trying to figure out what the original reason for introducing the code that is problematic is. The entire point is on projects big enough, you may not be able to just "follow the logic" enough to know that your fix the to apparent bug isn't re-introducing some regression that was fixed previously. So bisect to me, is a way to figure out where and why the code was changed the way that it was, so I have a better understanding of the changes I can make going forward. That doesn't mean the change made in the past was correct and must be maintained (obviously something is broken), or that it might be that the obvious solution based on following the code logic is correct. But that doesn't mean I wasted time making sure I best understand the reasoning behind the changes made. But this is also why I don't resort to it very often, because it isn't necessary in all cases (I'd even say it isn't necessary in most cases). Bisect also allows you to see other changes made in the commit in question, and around that commit, so you get a better overall picture of the logic.
        5y ago
- Rapzid 5y ago
  I agree with you in part, depending on the project, if that means just squashing it down first. This at least can keep the integration branch somewhat sane and without a bunch of intermediate commits that are junk and won't build. For this reason I prefer people to rebase on the integration branch instead of merging it in to stay current; unless there is a reason not to. And a lot of open source projects require PRs that to be pre-squashed or be able to be squashed before merging! However, there is a time and place still for deliberately crafting the PR commits. For instance if you are wanting the reviewer to be able to review chunks of the PR in isolation/stages, or if you want each of the commits to be buildable.. I suppose also for review/testing. This all depends on the project and other factors like the wider org.
- gregmac 5y ago
  > A complete and utter waste of time. You spend more time messing about with rebase than solving problems. Unless you're using a garbage client (eg, the git CLI) a rebase to get rid of the "typo" "oops" type commits (when you forgot something) takes I'd say 10-20 seconds. Most commonly I do this when I have several changes on the go at once and forget to commit a fixed unit test, new import, or something like that. I usually realize it right away, so my commit will usually be "add test" or "amend 2 commits ago" or something to remind myself which one it goes to (and it sticks out, because my other commits are usually in the format "PROJ-1234: Add new --host command-line option"). > When you're digging through VCS history due to a bug you often ignore the commit message anyway - if the code did what it seemed to do you wouldn't be there. I presume this is doing a bisect or something to narrow down where a bug was introduced? My experience comes from the opposite side. Usually I identify the line of code causing a bug, it makes me go "wtf, what is this even supposed to be doing?" so I do a git blame. Hopefully the commit message is useful and ideally leads me back to the original bug/ticket, so I can figure out what the original intent/fix was, rather than inadvertently breaking something (or re-causing a different bug it fixed). I'll also note that in code that is well-commented and well-tested, this step is rarely necessary. As an example, say I come across something like this: if (port > 443) ignoreErrors = true; Clearly, this was added to solve some some problem, but even without any context it's pretty obviously a bad solution to whatever that problem is. Git blame lets me go back to see what the original reason/bug was, evaluate if it's still relevant and then either fix it properly or safely remove this line.
  - FalconSensei 5y ago
    > My experience comes from the opposite side. Usually I identify the line of code causing a bug, it makes me go "wtf, what is this even supposed to be doing?" so I do a git blame. If the commit message is something like: PROJECT-TICKETNUMBER Description, you can you know... see the ticket/issue along with full details and discussions?
  - sethherr 5y ago
    > I'll also note that in code that is well-commented and well-tested, this step is rarely necessary. This is the most important line from this comment. If I'm using blame or bisect, it means I did something wrong. I merge 10 times a day - and if it takes me a minute, because I'm slower than you - that means it's ten minutes a day. Add the cognitive load of the whole process and the added time of explaining it to other team members - 10 minutes is an underestimate. I'm going to spend that time writing tests and comments, since even you note that they're the better option. Squash is good enough. I spend my energy improving the right things.
- globular-toast 5y ago
  > A complete and utter waste of time. You spend more time messing about with rebase than solving problems. You won't think that when customers report a regression that was introduced some time in the last year.
- tharkun__ 5y ago
  You have apparently never had the benefit of a properly run git repository. Our master has straight history. We always rebase to merge a PR and we have build scripts that check this as well. You will not be able to merge a PR that tries to get more than one commit onto master, if the commits touch more than one of the modules (we have a monorepo with lots of individual services). Until the PR has been approved we leave individual commits on the branch though as that can be helpful. I have not had to "mess" with rebases ever. Most rebases apply cleanly and if they don't the conflict resolution is mostly very easy. The 'hardest' conflicts to solve were actually in the end the easiest to solve, because all you had to do was to `git rebase --skip` the appropriate commits. Of course some developers that didn't know better spent hours trying to do those conflict resolutions and complained loudly. Fortunately they were that loud, so we caught it and just did the skipping. Everything applied cleanly without any conflicts after that. It's pretty awesome to have one commit per ticket on master and especially if something goes wrong. I can easily `git bisect` to find which ticket introduced a bug or a regression and I don't ever have to mess around. All commits have to have a ticket number in the commit message. It also enables firefighters to almost mechanically solve production issues. Prod is broken after the last production deploy? There's very very likely only one new commit since the last deploy before that (or maybe a few). In any case, just revert to the commit of the previous deploy, lock down the deployments and now you have time to properly solve the issue without time pressure. If there was more than one commit and you can pinpoint which of them caused the issue it would even be very easy to just revert that one commit and let deployments continue. No dealing with 27 commits that make up one ticket. Know thy tools!
  - u801e 5y ago
    > You will not be able to merge a PR that tries to get more than one commit onto master What value does the merge commit in the PR provide if a PR only has a single commit? Couldn't you essentially halve the number of commits in the commit history by amending the single PR commit message to contain the text that the merge commit will contain anyway?
    - tharkun__ 5y ago
      In case it wasn't clear from my post, I am very all over the place with the word 'merge'. I usually use it to describe the concept. We don't ever have actual merge commits. We merge by rebasing and that then makes the 'merge' just a fast forward.
- u801e 5y ago
  > A complete and utter waste of time. You spend more time messing about with rebase than solving problems. That can be avoided by only staging and committing changes when the change is actually completely implemented. For example, I'll make a change and then I'll stage it as follows: 1. Stage and commit a new method and associated unit tests 2. Stage and commit calls to the new method 3. Stage and commit the version update Having to go through extensive rebasing is really an issue with treating version control as a back up system rather than a way to record individual units of work with well crafted commit messages that describe what and why the change was made.
edoceo 5y ago
maybe its not 100% one way or the other? I love squash for somethings, but not others today I brought in one big merge, 20+ commits , so don't squash it. but I also have 20 "little fix" branches, with 1,2 commits each which I merge all together and squash in as one merge to main.
- sevencolors 5y ago
  This is exactly how i run my project. Amazing that folks get into these "two sides" debate. The reality is always a grey area. And you should be flexible enough to see the benefits of the different variations. With some guidelines on how to make a decision.
- PaulDavisThe1st 5y ago
  hah, a big merge. in the next month or so, i will need to merge a branch with nearly 500 commits that change more that 15k lines of code, under development for more than a year. but yeah, like your "one big merge", i do not intend to squash it (though there are actually some arguments in favor that only kick on with a merge of this size).
  - tharkun__ 5y ago
    I would argue that this feature branch of yours is way too long lived :) But I get it, sometimes you can't change some things in a corporate environment. I would like to know though, if those 500 commits are all individual PRs/tickets that originally might have been 3798 commits but each PR that got merged into the feature branch was squashed? Or are these 500 individual commits that are potentially interleaved, meaning commit #246 is for ticket ABC, commit #247 to #251 is ticket XYZ, #252 to #255 is ticket ABC again etc.? I'm basically re-commenting here but I think if the 500 commits are commits that if you didn't have to have a long-lived feature branch for some reason you would've made those exact 500 commit to master using squash-merge, then it's fine to rebase the feature branch on master and do a fast-forward 'merge' that makes all of those 500 commits suddenly appear on master (find except for the fact that you had _one year_ feature branch :))
eximius 5y ago
If only there was a way to have our cake and eat it too. I'm assuming there is because I'm assuming those squashed commits are floating somewhere in the reflog and somehow connected to the squashed commits in a way that should still let you bisect if only we had a tool to make it not suck. Alas, maybe someone with greater git-fu can inform us.
- waffletower 5y ago
  There are, as I answered separately: `git log -p --first-parent`
- rubyist5eva 5y ago
  In the article he says he keeps the original branch with all of the history around after they squash them with a reference to the original PR in the squashed merge commit message. So you can always just checkout the original branch and go digging in the full history.
  - waffletower 5y ago
    Unwieldy to keep branches around even on moderate sized teams/projects. You don't need them to have a `squashed` view of history when needed. I am surprised that developers still cling to this outdated squashing regimen when pull request tools found on github, gitlab, bitbucket etc. already provide synthetic squash views derived from atomic commits by default.
    - rubyist5eva 5y ago
      How is it unwieldy? They don't even show up locally when you `git branch` unless you've checked them out. Otherwise they just sit there doing nothing, there is literally no maintenance required on them once they've been squashed into mainline. Most central repositories end up having hundreds (or even thousands) of "finished" branches that nobody ever looks at anyway.
      - waffletower 5y ago
scott0129 5y ago
I would agree with your points if not for the fact that "clean up commits" or "typo fix" is a necessary result of PR's. Your teammate will request changes in your code, and the only way to cleanly communicate "yes I made that change, and ONLY that change" is through these clean-up commits. Otherwise, if you amend/force-push or open an entirely new PR, 99% of the diff are things that your team has already seen and reviewed. Squash merges let you clearly communicate how you addressed PR comments, while also keeping the master history clean.
- jcelerier 5y ago
  > Otherwise, if you amend/force-push or open an entirely new PR, 99% of the diff are things that your team has already seen and reviewed. gerrit has solved this issue for years by showing the diffs between each successive revision of a patch. e.g. look here the files at different origin patchsets : https://codereview.qt-project.org/c/qt/qtwayland/+/321246/3..6 https://codereview.qt-project.org/c/qt/qtwayland/+/321246/3....
  - jonhohle 5y ago
    This was a feature of ReviewBoard as well. The history of code review changes was maintained by the tool separately from commit history.
  - pjc50 5y ago
    It took me a while to get a workflow that actually works with Gerrit, and I occasionally think of trying to do some kind of autosquash so I can have successive commits locally. In practice I just use --amend all the time. Careful use of setting upstream and of course pull=rebase makes keeping up with trunk manageable.
  - servilio 5y ago
    You can also use git-range-diff[1]. [1] http://git-scm.com/docs/git-range-diff http://git-scm.com/docs/git-range-diff
    - 5y ago
ginja 5y ago
I do agree that PRs should have a clean history. However: - Services like GitHub allow you to restore the original branch, so you never actually lose history. So I don't see any major drawbacks of squashing on merge. - If your PRs are several thousand lines long, they probably should've been broken up into multiple PRs (your reviewer will appreciate it)
- finnh 5y ago
  Spot-on for both counts. Preserving a PR's individual commits in the master branch is insane - many of those commits won't represent a fully working system anyway, so why keep them in master?
  - sjoruk 5y ago
    > many of those commits won't represent a fully working system anyway This is entirely within the control of the developer(s) working on the project. Whether it's worth it is up to the people working on it. It's certainly not insane though - it's much easier to fix merge/rebase conflicts when each commit is small and easy to reason about.
    - finnh 5y ago
      > it's much easier to fix merge/rebase conflicts when each commit is small and easy to reason about. I agree, but I'd say it in the context of PRs. It's much easier to fix issues (and avoid them) when _PRs_ are small & easy to reason about.
  - gouggoug 5y ago
    It's entirely not insane _on the premise that the history is clean_. That's the whole argument that is being made here. If you think that "Preserving a PR's individual commits in the master branch is insane - many of those commits won't represent a fully working system anyway", your issue _is_ "that your commits don't represent a fully working system anyway", _that_ is what you need to fix. You need to make sure your history _does_ represent multiple small digestible atomic chunk of codes, that are understandable individually. By squashing, you're not fixing the root cause, you're treating the symptoms.
FunnyLookinHat 5y ago
> Squash merge has the major disadvantage of getting rid of valuable meaningful git history. I think the main point of the article is that the Git history is often *not* valuable (because it's hard to enforce good commit messages). Your later points are very valid, however, and that's why I argue for smaller pull requests (and never merging a PR into main that leaves it in a broken state).
jtchang 5y ago
> Squash merge has the major disadvantage of getting rid of valuable meaningful git history. There are reasons against it but the real world is messy. Developers tend to commit things, recommit, undo, redo, move things around. Is this valuable history? It can be but I would say 99% of the time it's more valuable to have a good commit message about what was intended rather than what actually happened. The idea of squash merge is to make the history clean from the get-go as you said.
- Izkata 5y ago
  > Developers tend to commit things, recommit, undo, redo, move things around. I see 2-3 distinct final commits in that list there, ones that should be separate and not squashed into one: * commit, recommit, undo (squash these ones, and depending on how much "undo" does could eliminate this entirely) * redo (what exactly is in this one depends on how much "undo" did in the prior, and how the "redo" was done) * move things around (feature is seemingly already working and this is a distinct separate "improve the code" step) > Is this valuable history? Keeping that last one separate is absurdly valuable when trying to figure out why the code acts the way it does, when a bug is reported later. A lot of bugs I encounter were in those steps when someone makes a simple typo moving the code around, and having it in the history makes it obvious it wasn't an intended change.
asimpletune 5y ago
Hey, so this POV comes up a lot, and I have to say that I think it mistakes how git commits should work, but I’ll add that you can sort of have both. First, in a production branch, git commits should be thought of as functions. Like “Apply commit X, get feature Y, unapply it and you get the reverse”. So the problem with preserving full git history in master is that it breaks that invariant. You have to sort of do like a range of commits, but then that doesn’t really always work because often times other commits can be interleaved into yours. To understand how I mean, just look at Linux or open source software projects, where the technical experts are more or less gatekeepers and aren’t accountable to any other influence. You’ll find that commits work this way, and their history is squashed. (Maintainers will also force you to rebase before merging and basically put all the work on you to get the PR in ship shape, which is a lot different from a corporate environment) Ok, but then to your point about preserving valuable, more granular commits, well, the solution is you just leave that remote branch up, either in your fork or elsewhere (but probably in your fork). These branches should have formal names identifying a ticket (at work) or an RFC or whatever, so it should be easy for people to discover what happened. They can see this remote issued a PR to this remote to implement RFC-123 or whatever, and they can go to that remote and see the granular commits if you preserved. Sorry, this is like a never ending debate and there are very strong opinions, but I sincerely think that in this situation there is actually a right answer. People who already know how to do this or are super familiar with open source, maintainers or team leads or whatever, I think don’t see the point in fighting over it, since they can just enforce whatever they want in their gatekeeper role. However, the truth is that there’s sort of a fundamental misunderstanding that the majority of the population has around git and I think it’s better to engage and explain when you get the chance.
- gouggoug 5y ago
  > First, in a production branch, git commits should be thought of as functions. Like “Apply commit X, get feature Y, unapply it and you get the reverse”. So the problem with preserving full git history in master is that it breaks that invariant. You have to sort of do like a range of commits, but then that doesn’t really always work because often times other commits can be interleaved into yours. Merge commits allow you to do exactly that. If you have a branch with, say 15 commits, you can merge that branch _with a merge commit_, preserving all 15 commits, but if you need to revert, you revert the merge, which automatically reverts all 15 commits. > Ok, but then to your point about preserving valuable, more granular commits, well, the solution is you just leave that remote branch up So you went through all the trouble of making your git history something clean and valuable. And now, instead of merging it as-is with a merge commit into main, you squash the entire history (that you literally just spent time making useful) into main. Now, because you think you might need this history (that you spent time making pretty and valuable), you come up with a whole process of creating a remote branch, giving it a name that follows specific conventions and whatnot, you also make the decision that you'll keep these branches forever, and, you need a way to link that remote branch to the squashed history. What is the point of this whole process? When you could simply merge your feature branch (with its pretty and useful history you spent time working on) in main and be done with it? I have a feeling this whole process you came up with stems from the fact that you don't seem to be aware that you can revert a merge commit.
cortesoft 5y ago
> When you squash merge a feature branch that has thousands of lines of code, and 6 months later you have a bug introduced by this feature branch, it becomes extremely hard to find which line introduced the bug. So, the only way that I can think that having the squash commit broken into individual commits would help to find the one broken line is because it would enable git bisect to find the failing commit. However, that implies that each individual commit inside that feature branch worked on its own. If that is the case, a better suggestion would be to break up the giant feature branch into smaller sub features that can be merged as they are completed
- deleted 5y ago
  [deleted]
- nemetroid 5y ago
  Even though most work can be split into a sequence of valid commits (i.e. sub-features), it's often not obvious how to best break up the larger feature before working on it.
  - cortesoft 5y ago
    Sure, but I don’t see how that changes the situation. There are three possibilities; you make a series of commits that are each complete and can run on their own, you make a series of commits that can’t be run on their own, or you make one big commit at the end with the full feature. If you are doing option 1, you could make one PR per commit, and each one could be squashed onto the main branch (although a single commit squash doesn’t do much). You can then use git bisect at a later time to find out which of those commits broke something. If you make a series of smaller, but non-functional commits, you won’t be able to use git bisect no matter if you squash them or not. If you squash them, git bisect will only tell you that the whole series of commits introduced a bug or not. If you don’t squash, git bisect won’t work because the build and/or tests will fail on the non-functional commits. You don’t gain better discoverability by not-squashing. The third option of one big commit makes squashing a moot point. Either way you only get one commit. My point is that you don’t get better discovery power by not squashing.
hinkley 5y ago
Squash merge is for people who are so wedded to the idea that you should guess why the code is the way that it is instead of looking it up, that they want to make sure nobody else can do it either. You know, assholes. It’s fine if you don’t want to use something, or use it, as long as it doesn’t drag your whole team into your decision. Which squash merge does.
deleted 5y ago
[deleted]
drtz 5y ago
> When you squash merge a feature branch that has thousands of lines of code, and 6 months later you have a bug introduced by this feature branch, it becomes extremely hard to find which line introduced the bug. The article makes an argument that each commit should be "a single logical change." Squashing thousands of lines of code from a long-running feature branches into a single commit is obviously not what the author is suggesting.
Cymen 5y ago
I am in agreement however I suspect many developers have never seen the usage of "git bisect" to track down a bug and fix it. Once you see the power of that, I think one can come to appreciate more the granular git history that is present when not using squash commits. Of course, git bisect still works with squash commits it just makes your job as the bug fixer much harder because typically squashed commits are quite large so you have to figure out what in the N lines of code introduced the defect.
- tharkun__ 5y ago
  I would beg to differ. How large your commits are, depends on your tickets. As I've just explained in another reply, squash merging with rebases does not have any direct correlation with how large your commits are or whether you can use long lived feature branches. We do sometimes use feature branches for example (very sparingly) but then that feature branch becomes a temporary 'master' that we squash merge to and the feature branch does _not_ get squashed but the individual commits suddenly all appear on master after rebasing, which results in a nice fast-forward 'merge'. Those feature branches are the result of multiple tickets which all have their own individual commits and which could all have made directly against master using the squash merge strategy, except there were 'reasons' not to (which I usually try to dispel and work with master directly but it's not always possible).
lifeisstillgood 5y ago
I think you are using got commit log as a form of documentation. I half-agree. I don't agree devs should spend (much) time cleaning up their logs as opposed to actually writing docs (inline) that help the next person I do agree that there is rarely a good (even half good) history of decisions. This is never the Jira / tickets kept outside of the system. But it is also no good abusing the commit log as a form of ... journal. (I know that sounds silly but ) I think an actual journal / blog kept by the dev lead will be much more useful
- oftenwrong 5y ago
  There's definitely value in putting documentation where it will be seen. For developers, writing in a code comment or commit message is a good bet. If there's an issue, they'll be in the code, and they'll be in the commit log, and they'll see what you've written. At times I've been asked to document things in a company wiki. Typically nobody reads it and it never gets updated when others make changes to the code.
bennysomething 5y ago
Disagree, having a bunch of commits made up of undo, redo , dunno what I'm doing , oh no now I know, commits is pretty annoying. Squash solves that (for me, when dealing with other peoples merges). Obviously just my opinion etc
- marton78 5y ago
  The problem is real and the parent commenter addresses it: learn interactive rebasing to craft a beautiful commit history.
streblo 5y ago
> Developers should be required to submit _clean_ PRs, that is, PR's whose git history has been organized and refactored in such a way that it removed "clean up commits", "typo fix", etc. The only way I've ever seen this accomplished is through Github squash and merge.
Ozzie_osman 5y ago
> When you squash merge a feature branch that has thousands of lines of code, and 6 months later you have a bug introduced by this feature branch, it becomes extremely hard to find which line introduced the bug. I try to avoid feature branches with "thousands of lines of code" on most of my teams, and have been pretty successful. Those types of feature branches create a lot of other problems. On the other hand, small, incremental pull requests that get merged back to master and have really short lifespans, along with things like feature flags to decouple delivering code from delivering functionality have worked really well. In this world, squash merges are awesome, because any squash merge is basically a "commit" in the other world, and developers can feel free to commit however they want within the branch.
garmaine 5y ago
> On the other hand, if you kept the history, and if this history was clean from the get go, it becomes easy to read the commits one-by-one and understand the issue. Or git-bisect.
- gouggoug 5y ago
  Yes, git bisect. But what does git bisect require to be useful? A set of small atomic commits that make it easy to identify which line of code might be the origin of a very subtle bug you're looking for. Squash+merge will not provide you with such history, and make bisecting much less useful.
codesnik 5y ago
Mandatory squash merge would be very bad, right. But it sounds like success of this policy is highly dependent on the code size of average PR. I really liked to work at place where commits were usually squash-rebased, which got rid of most "typo"s, but long lived huge feature branches lived mostly usual life. And if possible, some logically atomic and finished groundwork parts of feature branches were extracted and squash-rebased into master ahead of time, slimming feature branch, sometimes to the point that feature branch could be squash-rebased too. Git blame was VERY pleasant to work with, and git-bisect would actually work if need would arise.
- tharkun__ 5y ago
  Using squash-merge doesn't mean that you can't use long-lived feature branches if that's something that you need for whatever reason. It just means that you basically have a multi-stage squash merge strategy. We do this from time to time. We only allow rebased squash-merge to master. When we do have a long-lived feature branch for something then this feature branch basically becomes the master for the individual ticket branches and at the end, the feature branch is rebased onto master and merged as a fast forward. It can result in 50 commits appearing on master all at once but each of those commits is an individual small commit just as if they had been done directly with master and the squash-merge strategy. We rebase this feature branch on master very regularly too (once per sprint actually) to keep up to date with master. We also try not to do any long lived feature branches in the first place.
eplanit 5y ago
I see your points, but I disagree. FWIW, I also adopted the squash-merge only strategy with no regrets. > Squash merge has the major disadvantage of getting rid of valuable meaningful git history. It's not lost, at all -- it's in the branch it was squash-merged from. The point is that the topic/fix/experimental branch will likely have lots of history, including discarded ideas and other 'meanderings'. Every commit in that branch is meaningful in that it was a discrete step in that path. But, eventually what is _finally_ in that branch is what is relevant when merging to master. All that extra history in master would create confusion (and has for many in projects I've worked on). If the discipline is that every commit must be a meaningful, viable point _in master_, then a merge of the history would make sense -- but I've never seen that possible (or even desired) in practice.
MarkSweep 5y ago
As long as CI is run on all commits, I’ll agree. If you don’t run CI on every commit, then you have to balance nicer commits a higher likelihood of bisecting not working as easily.
chris_wot 5y ago
OpenOffice did what looks a bit like a "squash merge" way back in the day. It's a nightmare to work out where things got changed. I don't recommend it. I quite agree. Fix your commit history via an interactive rebase before you push. Learn to love fixup.

calpaterson 5y ago

> Developers should be required to submit _clean_ PRs, that is, PR's whose git history has been organized and refactored in such a way that it removed "clean up commits", "typo fix", etc. A complete and utter waste of time. You spend more time messing about with rebase than solving problems. When you're digging through VCS history due to a bug you often ignore the commit message anyway - if the code did what it seemed to do you wouldn't be there.

preordained 5y ago
Could not agree more...I wish I had more of substance to add, but an upvote didn't feel sufficient
qudat 5y ago
Agreed but I also think this depends heavily on context. If the organization cares a lot of clean commit messages, have a nice git history, and want PRs to be submitted in a clean state, great! This is not always the case. I've worked on codebases where we rarely scan the git history messages for anything useful, it's just as easy for us to dig into the code to figure out what's wrong.
dyeje 5y ago
It's a prime example of engineers mistaking aesthetics for utility.
tablespoon 5y ago
> A complete and utter waste of time. You spend more time messing about with rebase than solving problems. Eh. Not really. A clean history is a good resource for figuring out what was changed and why. Cleaning up history isn't even that much work anyway. > When you're digging through VCS history due to a bug you often ignore the commit message anyway - if the code did what it seemed to do you wouldn't be there. Only if the commit messages are shitty, which unfortunately they often are. I try to summarize what I'm changing and what I indent those changes to accomplish, because if there's one thing I hate doing, it's spending more time than necessary to puzzle out what I (or a coworker) was thinking at the time. VCS history can be a pretty powerful tool, but a messy history discourages the use of it.
- hbogert 5y ago
  amen! really, do people even use git bisect? If you do, you go through some rite of passage and I've seen people including myself make better (not perfect) commits. It remains a black art to gauge what the perfect amount of work in a commit should be.
throwaway1777 5y ago
Strongly disagree. Mainline branch commit history should be clean atomic commits that you can roll back. Otherwise you are just asking for trouble on any merge conflict or hotfix scenario.
pizza234 5y ago
> A complete and utter waste of time. You spend more time messing about with rebase than solving problems. I'm also one of those "micro committers". Because of this attitude, many hundreds pedantic commit rebases after, I'm now a better programmer (and I also spend next to no time with that type of rebases). This is because I have now a much higher capacity (and speed) of breaking down problems into smaller, self-contained, steps. There is a common understanding that VCs are just storages. VCs actually map the mental model of the developer. Having a precise, clear, granular VC history has a bidirectional relationship with being a precise, granular, clear developer.
mrkeen 5y ago
> You spend more time messing about with rebase than solving problems Rebase should be quick and straightforward. If it's not, you've got bigger problems. By rebasing, you're taking responsibility for changing the codebase as it exists now, rather than how it used to be.
sebastialonso 5y ago
I think I'm missing something here. How valuable is to have 20 commits of "fix this error" , "fix the fix of the error", "revert all fixes", "real fix".... etc? I'd argue a PR with many commits such as these, conveys little no no useful information, when the actual change is 1-3 LOC. what about cleaning and filtering out useless commits, by soft reseting the branch and commiting just the actual changes to merge. Sure, it can take a lot of time, but it's done once, by you, after all you're supplying the changes. And if it takes a really long time, you're doing it wrong in the first place by submitting PRs with many LOC. Have you tried rebasing branches with dozens of short useless commits? That's a real waste of time, and everyone needs to do this if you want to update your main branch. So you've now multiplied the amount of wasted time for everyone.
- darknavi 5y ago
  > what about cleaning and filtering out useless commits, by soft reseting the branch and commiting just the actual changes to merge. So... squashing?
  - phone8675309 5y ago
    Yes, but the author of the code, who would presumably know their intent, gets to choose how many commits to squash to instead of just one. This means that they can preserve the development process in a logical way.
  - u801e 5y ago
    What if the change really should be split into multiple commits. Having a mega commit affecting hundreds of lines across tens of files doesn't make reviewing the change easy, and reverting it will result in conflicts.
    - Seb-C 5y ago
      I would ask such changes to be split into multiple independent PRs
thom 5y ago
Do people here have examples of some bugs for which they had to resort to VCS history to find the cause? I'm struggling to picture a single bug in my whole career where this would have been quicker than just following the logic of the code. If there's information in commit messages that isn't evident in the code itself, that seems a terrible way to live.
- pizza234 5y ago
  As wrote in another comment, bisecting (which is for me a significant tool) relies on history (specifically, a granular one). However, it also must a disciplined history.
  - thom 5y ago
    What's an example of a bug you've had to bisect for recently? Forgive me, it just seems like such a last resort thing.
    - LocalPCGuy 5y ago
      I don't bisect regularly, but when I do, it's usually trying to figure out what the original reason for introducing the code that is problematic is. The entire point is on projects big enough, you may not be able to just "follow the logic" enough to know that your fix the to apparent bug isn't re-introducing some regression that was fixed previously. So bisect to me, is a way to figure out where and why the code was changed the way that it was, so I have a better understanding of the changes I can make going forward. That doesn't mean the change made in the past was correct and must be maintained (obviously something is broken), or that it might be that the obvious solution based on following the code logic is correct. But that doesn't mean I wasted time making sure I best understand the reasoning behind the changes made. But this is also why I don't resort to it very often, because it isn't necessary in all cases (I'd even say it isn't necessary in most cases). Bisect also allows you to see other changes made in the commit in question, and around that commit, so you get a better overall picture of the logic.
      - 5y ago
Rapzid 5y ago
I agree with you in part, depending on the project, if that means just squashing it down first. This at least can keep the integration branch somewhat sane and without a bunch of intermediate commits that are junk and won't build. For this reason I prefer people to rebase on the integration branch instead of merging it in to stay current; unless there is a reason not to. And a lot of open source projects require PRs that to be pre-squashed or be able to be squashed before merging! However, there is a time and place still for deliberately crafting the PR commits. For instance if you are wanting the reviewer to be able to review chunks of the PR in isolation/stages, or if you want each of the commits to be buildable.. I suppose also for review/testing. This all depends on the project and other factors like the wider org.
gregmac 5y ago
> A complete and utter waste of time. You spend more time messing about with rebase than solving problems. Unless you're using a garbage client (eg, the git CLI) a rebase to get rid of the "typo" "oops" type commits (when you forgot something) takes I'd say 10-20 seconds. Most commonly I do this when I have several changes on the go at once and forget to commit a fixed unit test, new import, or something like that. I usually realize it right away, so my commit will usually be "add test" or "amend 2 commits ago" or something to remind myself which one it goes to (and it sticks out, because my other commits are usually in the format "PROJ-1234: Add new --host command-line option"). > When you're digging through VCS history due to a bug you often ignore the commit message anyway - if the code did what it seemed to do you wouldn't be there. I presume this is doing a bisect or something to narrow down where a bug was introduced? My experience comes from the opposite side. Usually I identify the line of code causing a bug, it makes me go "wtf, what is this even supposed to be doing?" so I do a git blame. Hopefully the commit message is useful and ideally leads me back to the original bug/ticket, so I can figure out what the original intent/fix was, rather than inadvertently breaking something (or re-causing a different bug it fixed). I'll also note that in code that is well-commented and well-tested, this step is rarely necessary. As an example, say I come across something like this: if (port > 443) ignoreErrors = true; Clearly, this was added to solve some some problem, but even without any context it's pretty obviously a bad solution to whatever that problem is. Git blame lets me go back to see what the original reason/bug was, evaluate if it's still relevant and then either fix it properly or safely remove this line.
- FalconSensei 5y ago
  > My experience comes from the opposite side. Usually I identify the line of code causing a bug, it makes me go "wtf, what is this even supposed to be doing?" so I do a git blame. If the commit message is something like: PROJECT-TICKETNUMBER Description, you can you know... see the ticket/issue along with full details and discussions?
- sethherr 5y ago
  > I'll also note that in code that is well-commented and well-tested, this step is rarely necessary. This is the most important line from this comment. If I'm using blame or bisect, it means I did something wrong. I merge 10 times a day - and if it takes me a minute, because I'm slower than you - that means it's ten minutes a day. Add the cognitive load of the whole process and the added time of explaining it to other team members - 10 minutes is an underestimate. I'm going to spend that time writing tests and comments, since even you note that they're the better option. Squash is good enough. I spend my energy improving the right things.
globular-toast 5y ago
> A complete and utter waste of time. You spend more time messing about with rebase than solving problems. You won't think that when customers report a regression that was introduced some time in the last year.
tharkun__ 5y ago
You have apparently never had the benefit of a properly run git repository. Our master has straight history. We always rebase to merge a PR and we have build scripts that check this as well. You will not be able to merge a PR that tries to get more than one commit onto master, if the commits touch more than one of the modules (we have a monorepo with lots of individual services). Until the PR has been approved we leave individual commits on the branch though as that can be helpful. I have not had to "mess" with rebases ever. Most rebases apply cleanly and if they don't the conflict resolution is mostly very easy. The 'hardest' conflicts to solve were actually in the end the easiest to solve, because all you had to do was to `git rebase --skip` the appropriate commits. Of course some developers that didn't know better spent hours trying to do those conflict resolutions and complained loudly. Fortunately they were that loud, so we caught it and just did the skipping. Everything applied cleanly without any conflicts after that. It's pretty awesome to have one commit per ticket on master and especially if something goes wrong. I can easily `git bisect` to find which ticket introduced a bug or a regression and I don't ever have to mess around. All commits have to have a ticket number in the commit message. It also enables firefighters to almost mechanically solve production issues. Prod is broken after the last production deploy? There's very very likely only one new commit since the last deploy before that (or maybe a few). In any case, just revert to the commit of the previous deploy, lock down the deployments and now you have time to properly solve the issue without time pressure. If there was more than one commit and you can pinpoint which of them caused the issue it would even be very easy to just revert that one commit and let deployments continue. No dealing with 27 commits that make up one ticket. Know thy tools!
- u801e 5y ago
  > You will not be able to merge a PR that tries to get more than one commit onto master What value does the merge commit in the PR provide if a PR only has a single commit? Couldn't you essentially halve the number of commits in the commit history by amending the single PR commit message to contain the text that the merge commit will contain anyway?
  - tharkun__ 5y ago
    In case it wasn't clear from my post, I am very all over the place with the word 'merge'. I usually use it to describe the concept. We don't ever have actual merge commits. We merge by rebasing and that then makes the 'merge' just a fast forward.
u801e 5y ago
> A complete and utter waste of time. You spend more time messing about with rebase than solving problems. That can be avoided by only staging and committing changes when the change is actually completely implemented. For example, I'll make a change and then I'll stage it as follows: 1. Stage and commit a new method and associated unit tests 2. Stage and commit calls to the new method 3. Stage and commit the version update Having to go through extensive rebasing is really an issue with treating version control as a back up system rather than a way to record individual units of work with well crafted commit messages that describe what and why the change was made.

edoceo 5y ago

maybe its not 100% one way or the other? I love squash for somethings, but not others today I brought in one big merge, 20+ commits , so don't squash it. but I also have 20 "little fix" branches, with 1,2 commits each which I merge all together and squash in as one merge to main.

sevencolors 5y ago
This is exactly how i run my project. Amazing that folks get into these "two sides" debate. The reality is always a grey area. And you should be flexible enough to see the benefits of the different variations. With some guidelines on how to make a decision.
PaulDavisThe1st 5y ago
hah, a big merge. in the next month or so, i will need to merge a branch with nearly 500 commits that change more that 15k lines of code, under development for more than a year. but yeah, like your "one big merge", i do not intend to squash it (though there are actually some arguments in favor that only kick on with a merge of this size).
- tharkun__ 5y ago
  I would argue that this feature branch of yours is way too long lived :) But I get it, sometimes you can't change some things in a corporate environment. I would like to know though, if those 500 commits are all individual PRs/tickets that originally might have been 3798 commits but each PR that got merged into the feature branch was squashed? Or are these 500 individual commits that are potentially interleaved, meaning commit #246 is for ticket ABC, commit #247 to #251 is ticket XYZ, #252 to #255 is ticket ABC again etc.? I'm basically re-commenting here but I think if the 500 commits are commits that if you didn't have to have a long-lived feature branch for some reason you would've made those exact 500 commit to master using squash-merge, then it's fine to rebase the feature branch on master and do a fast-forward 'merge' that makes all of those 500 commits suddenly appear on master (find except for the fact that you had _one year_ feature branch :))

eximius 5y ago

If only there was a way to have our cake and eat it too. I'm assuming there is because I'm assuming those squashed commits are floating somewhere in the reflog and somehow connected to the squashed commits in a way that should still let you bisect if only we had a tool to make it not suck. Alas, maybe someone with greater git-fu can inform us.

waffletower 5y ago
There are, as I answered separately: `git log -p --first-parent`
rubyist5eva 5y ago
In the article he says he keeps the original branch with all of the history around after they squash them with a reference to the original PR in the squashed merge commit message. So you can always just checkout the original branch and go digging in the full history.
- waffletower 5y ago
  Unwieldy to keep branches around even on moderate sized teams/projects. You don't need them to have a `squashed` view of history when needed. I am surprised that developers still cling to this outdated squashing regimen when pull request tools found on github, gitlab, bitbucket etc. already provide synthetic squash views derived from atomic commits by default.
  - rubyist5eva 5y ago
    How is it unwieldy? They don't even show up locally when you `git branch` unless you've checked them out. Otherwise they just sit there doing nothing, there is literally no maintenance required on them once they've been squashed into mainline. Most central repositories end up having hundreds (or even thousands) of "finished" branches that nobody ever looks at anyway.
    - waffletower 5y ago

scott0129 5y ago

I would agree with your points if not for the fact that "clean up commits" or "typo fix" is a necessary result of PR's. Your teammate will request changes in your code, and the only way to cleanly communicate "yes I made that change, and ONLY that change" is through these clean-up commits. Otherwise, if you amend/force-push or open an entirely new PR, 99% of the diff are things that your team has already seen and reviewed. Squash merges let you clearly communicate how you addressed PR comments, while also keeping the master history clean.

jcelerier 5y ago
> Otherwise, if you amend/force-push or open an entirely new PR, 99% of the diff are things that your team has already seen and reviewed. gerrit has solved this issue for years by showing the diffs between each successive revision of a patch. e.g. look here the files at different origin patchsets : https://codereview.qt-project.org/c/qt/qtwayland/+/321246/3..6 https://codereview.qt-project.org/c/qt/qtwayland/+/321246/3....
- jonhohle 5y ago
  This was a feature of ReviewBoard as well. The history of code review changes was maintained by the tool separately from commit history.
- pjc50 5y ago
  It took me a while to get a workflow that actually works with Gerrit, and I occasionally think of trying to do some kind of autosquash so I can have successive commits locally. In practice I just use --amend all the time. Careful use of setting upstream and of course pull=rebase makes keeping up with trunk manageable.
- servilio 5y ago
  You can also use git-range-diff[1]. [1] http://git-scm.com/docs/git-range-diff http://git-scm.com/docs/git-range-diff
  - 5y ago

ginja 5y ago

I do agree that PRs should have a clean history. However: - Services like GitHub allow you to restore the original branch, so you never actually lose history. So I don't see any major drawbacks of squashing on merge. - If your PRs are several thousand lines long, they probably should've been broken up into multiple PRs (your reviewer will appreciate it)

finnh 5y ago
Spot-on for both counts. Preserving a PR's individual commits in the master branch is insane - many of those commits won't represent a fully working system anyway, so why keep them in master?
- sjoruk 5y ago
  > many of those commits won't represent a fully working system anyway This is entirely within the control of the developer(s) working on the project. Whether it's worth it is up to the people working on it. It's certainly not insane though - it's much easier to fix merge/rebase conflicts when each commit is small and easy to reason about.
  - finnh 5y ago
    > it's much easier to fix merge/rebase conflicts when each commit is small and easy to reason about. I agree, but I'd say it in the context of PRs. It's much easier to fix issues (and avoid them) when _PRs_ are small & easy to reason about.
- gouggoug 5y ago
  It's entirely not insane _on the premise that the history is clean_. That's the whole argument that is being made here. If you think that "Preserving a PR's individual commits in the master branch is insane - many of those commits won't represent a fully working system anyway", your issue _is_ "that your commits don't represent a fully working system anyway", _that_ is what you need to fix. You need to make sure your history _does_ represent multiple small digestible atomic chunk of codes, that are understandable individually. By squashing, you're not fixing the root cause, you're treating the symptoms.

FunnyLookinHat 5y ago

> Squash merge has the major disadvantage of getting rid of valuable meaningful git history. I think the main point of the article is that the Git history is often *not* valuable (because it's hard to enforce good commit messages). Your later points are very valid, however, and that's why I argue for smaller pull requests (and never merging a PR into main that leaves it in a broken state).

jtchang 5y ago

> Squash merge has the major disadvantage of getting rid of valuable meaningful git history. There are reasons against it but the real world is messy. Developers tend to commit things, recommit, undo, redo, move things around. Is this valuable history? It can be but I would say 99% of the time it's more valuable to have a good commit message about what was intended rather than what actually happened. The idea of squash merge is to make the history clean from the get-go as you said.

Izkata 5y ago
> Developers tend to commit things, recommit, undo, redo, move things around. I see 2-3 distinct final commits in that list there, ones that should be separate and not squashed into one: * commit, recommit, undo (squash these ones, and depending on how much "undo" does could eliminate this entirely) * redo (what exactly is in this one depends on how much "undo" did in the prior, and how the "redo" was done) * move things around (feature is seemingly already working and this is a distinct separate "improve the code" step) > Is this valuable history? Keeping that last one separate is absurdly valuable when trying to figure out why the code acts the way it does, when a bug is reported later. A lot of bugs I encounter were in those steps when someone makes a simple typo moving the code around, and having it in the history makes it obvious it wasn't an intended change.

asimpletune 5y ago

Hey, so this POV comes up a lot, and I have to say that I think it mistakes how git commits should work, but I’ll add that you can sort of have both. First, in a production branch, git commits should be thought of as functions. Like “Apply commit X, get feature Y, unapply it and you get the reverse”. So the problem with preserving full git history in master is that it breaks that invariant. You have to sort of do like a range of commits, but then that doesn’t really always work because often times other commits can be interleaved into yours. To understand how I mean, just look at Linux or open source software projects, where the technical experts are more or less gatekeepers and aren’t accountable to any other influence. You’ll find that commits work this way, and their history is squashed. (Maintainers will also force you to rebase before merging and basically put all the work on you to get the PR in ship shape, which is a lot different from a corporate environment) Ok, but then to your point about preserving valuable, more granular commits, well, the solution is you just leave that remote branch up, either in your fork or elsewhere (but probably in your fork). These branches should have formal names identifying a ticket (at work) or an RFC or whatever, so it should be easy for people to discover what happened. They can see this remote issued a PR to this remote to implement RFC-123 or whatever, and they can go to that remote and see the granular commits if you preserved. Sorry, this is like a never ending debate and there are very strong opinions, but I sincerely think that in this situation there is actually a right answer. People who already know how to do this or are super familiar with open source, maintainers or team leads or whatever, I think don’t see the point in fighting over it, since they can just enforce whatever they want in their gatekeeper role. However, the truth is that there’s sort of a fundamental misunderstanding that the majority of the population has around git and I think it’s better to engage and explain when you get the chance.

gouggoug 5y ago
> First, in a production branch, git commits should be thought of as functions. Like “Apply commit X, get feature Y, unapply it and you get the reverse”. So the problem with preserving full git history in master is that it breaks that invariant. You have to sort of do like a range of commits, but then that doesn’t really always work because often times other commits can be interleaved into yours. Merge commits allow you to do exactly that. If you have a branch with, say 15 commits, you can merge that branch _with a merge commit_, preserving all 15 commits, but if you need to revert, you revert the merge, which automatically reverts all 15 commits. > Ok, but then to your point about preserving valuable, more granular commits, well, the solution is you just leave that remote branch up So you went through all the trouble of making your git history something clean and valuable. And now, instead of merging it as-is with a merge commit into main, you squash the entire history (that you literally just spent time making useful) into main. Now, because you think you might need this history (that you spent time making pretty and valuable), you come up with a whole process of creating a remote branch, giving it a name that follows specific conventions and whatnot, you also make the decision that you'll keep these branches forever, and, you need a way to link that remote branch to the squashed history. What is the point of this whole process? When you could simply merge your feature branch (with its pretty and useful history you spent time working on) in main and be done with it? I have a feeling this whole process you came up with stems from the fact that you don't seem to be aware that you can revert a merge commit.

cortesoft 5y ago

> When you squash merge a feature branch that has thousands of lines of code, and 6 months later you have a bug introduced by this feature branch, it becomes extremely hard to find which line introduced the bug. So, the only way that I can think that having the squash commit broken into individual commits would help to find the one broken line is because it would enable git bisect to find the failing commit. However, that implies that each individual commit inside that feature branch worked on its own. If that is the case, a better suggestion would be to break up the giant feature branch into smaller sub features that can be merged as they are completed

deleted 5y ago
[deleted]
nemetroid 5y ago
Even though most work can be split into a sequence of valid commits (i.e. sub-features), it's often not obvious how to best break up the larger feature before working on it.
- cortesoft 5y ago
  Sure, but I don’t see how that changes the situation. There are three possibilities; you make a series of commits that are each complete and can run on their own, you make a series of commits that can’t be run on their own, or you make one big commit at the end with the full feature. If you are doing option 1, you could make one PR per commit, and each one could be squashed onto the main branch (although a single commit squash doesn’t do much). You can then use git bisect at a later time to find out which of those commits broke something. If you make a series of smaller, but non-functional commits, you won’t be able to use git bisect no matter if you squash them or not. If you squash them, git bisect will only tell you that the whole series of commits introduced a bug or not. If you don’t squash, git bisect won’t work because the build and/or tests will fail on the non-functional commits. You don’t gain better discoverability by not-squashing. The third option of one big commit makes squashing a moot point. Either way you only get one commit. My point is that you don’t get better discovery power by not squashing.

hinkley 5y ago

Squash merge is for people who are so wedded to the idea that you should guess why the code is the way that it is instead of looking it up, that they want to make sure nobody else can do it either. You know, assholes. It’s fine if you don’t want to use something, or use it, as long as it doesn’t drag your whole team into your decision. Which squash merge does.

deleted 5y ago

[deleted]

drtz 5y ago

> When you squash merge a feature branch that has thousands of lines of code, and 6 months later you have a bug introduced by this feature branch, it becomes extremely hard to find which line introduced the bug. The article makes an argument that each commit should be "a single logical change." Squashing thousands of lines of code from a long-running feature branches into a single commit is obviously not what the author is suggesting.

Cymen 5y ago

I am in agreement however I suspect many developers have never seen the usage of "git bisect" to track down a bug and fix it. Once you see the power of that, I think one can come to appreciate more the granular git history that is present when not using squash commits. Of course, git bisect still works with squash commits it just makes your job as the bug fixer much harder because typically squashed commits are quite large so you have to figure out what in the N lines of code introduced the defect.

tharkun__ 5y ago
I would beg to differ. How large your commits are, depends on your tickets. As I've just explained in another reply, squash merging with rebases does not have any direct correlation with how large your commits are or whether you can use long lived feature branches. We do sometimes use feature branches for example (very sparingly) but then that feature branch becomes a temporary 'master' that we squash merge to and the feature branch does _not_ get squashed but the individual commits suddenly all appear on master after rebasing, which results in a nice fast-forward 'merge'. Those feature branches are the result of multiple tickets which all have their own individual commits and which could all have made directly against master using the squash merge strategy, except there were 'reasons' not to (which I usually try to dispel and work with master directly but it's not always possible).

lifeisstillgood 5y ago

I think you are using got commit log as a form of documentation. I half-agree. I don't agree devs should spend (much) time cleaning up their logs as opposed to actually writing docs (inline) that help the next person I do agree that there is rarely a good (even half good) history of decisions. This is never the Jira / tickets kept outside of the system. But it is also no good abusing the commit log as a form of ... journal. (I know that sounds silly but ) I think an actual journal / blog kept by the dev lead will be much more useful

oftenwrong 5y ago
There's definitely value in putting documentation where it will be seen. For developers, writing in a code comment or commit message is a good bet. If there's an issue, they'll be in the code, and they'll be in the commit log, and they'll see what you've written. At times I've been asked to document things in a company wiki. Typically nobody reads it and it never gets updated when others make changes to the code.

bennysomething 5y ago

Disagree, having a bunch of commits made up of undo, redo , dunno what I'm doing , oh no now I know, commits is pretty annoying. Squash solves that (for me, when dealing with other peoples merges). Obviously just my opinion etc

marton78 5y ago
The problem is real and the parent commenter addresses it: learn interactive rebasing to craft a beautiful commit history.

streblo 5y ago

Ozzie_osman 5y ago

> When you squash merge a feature branch that has thousands of lines of code, and 6 months later you have a bug introduced by this feature branch, it becomes extremely hard to find which line introduced the bug. I try to avoid feature branches with "thousands of lines of code" on most of my teams, and have been pretty successful. Those types of feature branches create a lot of other problems. On the other hand, small, incremental pull requests that get merged back to master and have really short lifespans, along with things like feature flags to decouple delivering code from delivering functionality have worked really well. In this world, squash merges are awesome, because any squash merge is basically a "commit" in the other world, and developers can feel free to commit however they want within the branch.

garmaine 5y ago

> On the other hand, if you kept the history, and if this history was clean from the get go, it becomes easy to read the commits one-by-one and understand the issue. Or git-bisect.

gouggoug 5y ago
Yes, git bisect. But what does git bisect require to be useful? A set of small atomic commits that make it easy to identify which line of code might be the origin of a very subtle bug you're looking for. Squash+merge will not provide you with such history, and make bisecting much less useful.

codesnik 5y ago

Mandatory squash merge would be very bad, right. But it sounds like success of this policy is highly dependent on the code size of average PR. I really liked to work at place where commits were usually squash-rebased, which got rid of most "typo"s, but long lived huge feature branches lived mostly usual life. And if possible, some logically atomic and finished groundwork parts of feature branches were extracted and squash-rebased into master ahead of time, slimming feature branch, sometimes to the point that feature branch could be squash-rebased too. Git blame was VERY pleasant to work with, and git-bisect would actually work if need would arise.

tharkun__ 5y ago
Using squash-merge doesn't mean that you can't use long-lived feature branches if that's something that you need for whatever reason. It just means that you basically have a multi-stage squash merge strategy. We do this from time to time. We only allow rebased squash-merge to master. When we do have a long-lived feature branch for something then this feature branch basically becomes the master for the individual ticket branches and at the end, the feature branch is rebased onto master and merged as a fast forward. It can result in 50 commits appearing on master all at once but each of those commits is an individual small commit just as if they had been done directly with master and the squash-merge strategy. We rebase this feature branch on master very regularly too (once per sprint actually) to keep up to date with master. We also try not to do any long lived feature branches in the first place.

eplanit 5y ago

I see your points, but I disagree. FWIW, I also adopted the squash-merge only strategy with no regrets. > Squash merge has the major disadvantage of getting rid of valuable meaningful git history. It's not lost, at all -- it's in the branch it was squash-merged from. The point is that the topic/fix/experimental branch will likely have lots of history, including discarded ideas and other 'meanderings'. Every commit in that branch is meaningful in that it was a discrete step in that path. But, eventually what is _finally_ in that branch is what is relevant when merging to master. All that extra history in master would create confusion (and has for many in projects I've worked on). If the discipline is that every commit must be a meaningful, viable point _in master_, then a merge of the history would make sense -- but I've never seen that possible (or even desired) in practice.

MarkSweep 5y ago

As long as CI is run on all commits, I’ll agree. If you don’t run CI on every commit, then you have to balance nicer commits a higher likelihood of bisecting not working as easily.

chris_wot 5y ago

OpenOffice did what looks a bit like a "squash merge" way back in the day. It's a nightmare to work out where things got changed. I don't recommend it. I quite agree. Fix your commit history via an interactive rebase before you push. Learn to love fixup.