10 ms·

> I like squash commits: build a feature iteratively in a branch + pull requests, squash to master once it has the tests and documentation bundled together with

> I like squash commits: build a feature iteratively in a branch + pull requests, squash to master once it has the tests and documentation bundled together with the implementation. This gives you linear history I disagree with squashing commits when merging to master, here's why: Your commit history is what you will use later down the line (think 6 months, a year, 2 years later) to find a very nasty yet incredibly subtle bug in your application. Imagine said bug was introduced by a feature that took about 1 full month to code, introducing 1000 lines and removing 500. That feature was probably made of many small commits (say, 25 commits total), on a feature branch called "super-feature". If your developer cared at all about making "atomic" meaningful commits, each one of those commit will be introducing a small very well defined subset of the entire "super-feature". The "super-feature" will in the end be the result of all of those commits together. Squashing that history of 25 commits, will create one giant commit of 1500 changes. Have fun finding where in those 1500 changes that little nasty bug was introduced! Instead, if you hadn't squashed, you simply use `git bisect` and find in no time that the bug was introduced by the 12th commit of that branch, which itself was a small commit of 1 little change: diff --git a/statistics.php b/statistics.php index 69e435aa9f..46666daa93 100644 --- a/statistics.php +++ b/statistics.php @@ -8,7 +8,7 @@ - a = b + c + a = b - c If you want a very linear history, enforce a: git rebase master super-feature git merge super-feature --no-ff This will rebase your branch on top of master, but, when merging, the `--no-ff` option will tell git to create a merge commit. This merge commit will make it very clear that the code branched off and was later merged back into master, yet, it will keep the history linear. ======= TL;DR: Your commit history is as important as your code! Don't squash it away! I wish github never introduced that feature and instead developers were taught how to create a meaningful history of commits. =======

nickysielicki 7y ago
> Your commit history is what you will use later down the line (think 6 months, a year, 2 years later) to find a very nasty yet incredibly subtle bug in your application. No, it won’t. If I didn’t catch it then, I will have to read the code and understand it and spot it. At work, where my builds take anywhere from 30 minutes to 3 hours depending on the target, your git bisect on 1500 commits represents days of building and testing. Or I could just read the code again. Commit messages should provide context for going through the process of rereading the code, and little else. If a commit message literally describes what the code does (“change constant from X to Y”), it’s self-describing and unnecessary.
- idoubtit 7y ago
  It's okay if you really prefer to read a huge squashed commit instead of an atomic commit in a branch, but you made it sound like squashing was necessary in order to reduce building and testing. That's wrong. You can use `git bisect` on merge commits only, then, once the faulty merge is detected, bisect the commits of the merged branch. Bisecting merges won't take more time than bisecting squashed branches.
pbh101 7y ago
Agree with the thoughts on atomic and meaningful commits (though would drop the merge commit entirely), and I will rebase my commit history extensively at times to get it into a presentable state along the lines listed here. Bottom line: the history, content, and message of each commit, in addition to the resultant repo state, are all artifacts of your work and are worth shaping and crafting in order to optimize for communicating to yourself and your fellow developers in the future.
- gouggoug 7y ago
  > though would drop the merge commit entirely The merge commit is important though. It helps delimiting a set of commits that have meaning all together from the rest of your commit history. Here's an example: a -> b -> c -> d -> e -> f -> g -> h -> i -> j -> k In the history above, did you notice that commits c through h are actually related to one another? They were introduced a while ago by a feature branch. Compare the previous history to this: c -> d -> e -> f -> g -> h a -> b / \ i -> j -> k Now that's very clear that c through h are part of the same line of work. All those commits were probably part of 1 single pull request and were reviewed in the context of one another. My history here is still very linear. Yes, there's 1 little branching off, but that's all. At my previous company we had a codebase of well over 20k commits entirely made of those little branches branching off master, then merging back in. It made it for an incredibly readable history. An other added advantage of not removing the merge commit is that you can revert the entire merge of your branch in one go. Without the merge commit, you need to remove each commit one by one (and that's granted you remember which commit was the first one and which one was the last one).
  - pbh101 7y ago
    Agreed your history is still linear. But I would suggest your merge commits are perhaps more trouble than they are worth. In commit sequences we prefix almost all commit subjects with the JIRA ticket ID, so the sequenced tickets are quite clearly marked as being related to each other without incurring the merge commit cost. As for reverting merge commits, I'd rather not personally have to remember the rules as to how to re-apply after the revert (revert the merge revert, then merge the branch again)[1], let alone working on spreading that knowledge losslessly across the organization. This is admittedly something I haven't done much of, but I'd rather revert ~3-9 individual commits, squash the reversion commits (usually with a bit more context on why we're reverting), and push. And it could be scripted. I haven't made a script for it as it doesn't happen that often, but between using rev-parse and a sed script in the GIT_EDITOR environment variable it would be pretty quick. Having merge commits also means that you have to teach everyone how to view diffs of merge commits, which admittedly isn't that hard, but it is still just one more barrier to know/teach/overcome, and the default behavior of showing a zero-line diff for a no-op merge commit doesn't help. [1] https://mirrors.edge.kernel.org/pub/software/scm/git/docs/howto/revert-a-faulty-merge.txt https://mirrors.edge.kernel.org/pub/software/scm/git/docs/ho...
bArray 7y ago
> Squashing that history of 25 commits, will create one giant > commit of 1500 changes. Have fun finding where in those > 1500 changes that little nasty bug was introduced! 100% agree, the only purpose of this is to make "pretty graphs" at the cost of destroying the history of the code changes. I prefer to write small commits with simple messages that explain exactly what that change does. Sometimes it costs some time in Git Gui selecting the lines that make up the commit, but ultimately it's worth it.
- u801e 7y ago
  > I prefer to write small commits with simple messages that explain exactly what that change does. One thing people should include in commit messages is why the change is being made. In other words, what's the purpose of the change. That can provide a lot of context that can be lost over time. So my commit messages include both what was done and why it was done. The why part can range from things like this method is needed for this feature that does something, or this fixes a bug that was causing this issue, or this other feature is no longer needed in context of this other reason, etc.
  - mcv 7y ago
    What can also help is linking to the related Jira ticket (or whichever issue tracking system you use). And separate "cleanup" commits from commits that make meaningful changes. A commit that changes every line in a file because it's fixing incorrect indenting is fine. A commit that does that but also introduces a small functional change, makes that change invisible.
    - u801e 7y ago
      We've actually changed tracking systems over the years (meaning that the link in the commit message is no longer valid). So, it's best to include both the link and the actual text. > A commit that does that but also introduces a small functional change, makes that change invisible. While it's sometimes possible to separate the actual change from the formatting change by ignoring whitespace if the diff, it's definitely better to make each type of change in a separate commit.
Silhouette 7y ago
You have made a case for keeping the work-in-progress commits. The case the other way is that what should matter at the end of the feature development is the visible contribution it makes to the overall project, i.e., the finished code, tests, etc. This has two fundamental consequences. Firstly, in this model, your final commit that goes onto whatever you call your primary development branch should be a complete, self-contained piece of work, like adding a new feature or fixing some specific piece of broken behaviour. It should include any related tests, data files, etc. It should be something that can be reviewed in isolation and make sense. It should ideally be something that has minimal interactions with any other code that aren't strictly necessary, so it can be removed or replaced later if necessary with minimal effort to resolve or rewrite anything else that has happened since. Secondly, requiring all work-in-progress commits to also be meaningful and live forever in your shared repo significantly limits the usefulness of source control for the developer during the development stage. They might be experimenting with different approaches but want to keep track of a version where some specific aspect of the code is currently working. They might be about to refactor to clean some new code up but want to make sure they can't lose the exact version they had just got working due to unintended side effects. They don't want to be getting every individual commit code-reviewed or run any slow testing processes, just in case someone later comes along and cherry-picks that particular point in history to use as the basis for something else. So I will respectfully disagree with you that the full commit history at this level is as important as the final code. My reason for that is that the final code is complete, tested, reviewed, and going into the finished product, while anything in the work-in-progress commits that didn't survive to that point was determined, by the developer doing all of that work, at the time, with the aid of all of the testing and reviews and tools that were involved, not to be as valuable as the final version. If there really were interesting alternatives considered during development that might be useful to keep around for later reference, you can always tag or branch at that specific point and give it a meaningful name. However, I would suggest that this is like keeping issues open in your bug tracker when you know you'll never fix them: unless you also have some process for keeping track of such ideas and why they are valuable to refer back to them and take some useful action later, you're better off not having them clutter your main development process and tools forever.
- gouggoug 7y ago
battery_cowboy 7y ago
But can't you just go back and reopen that branch? Just because you squash-merge the branch to master doesn't mean you can't keep the branch archived. I have seen where you basically just "rename" the branch (i.e.: make a new branch of the branch and just leave it as-is) to "merged-2020-03-03-BRANCHNAME" or something and then you don't touch it. Meanwhile, your master is a neat and tidy history of features added, tickets finished, and bugs fixed without all of the "fixed the thing I forgot to add" messages that inevitably sneak their way into your git log.
- tremon 7y ago
  without all of the "fixed the thing I forgot to add" messages This is why you use rebase --interactive or other ways of amending commits before submitting a PR. It is not an argument for commit squashing.
  - battery_cowboy 7y ago
    > rebase --interactive like 90% of developers (maybe 95%) don't even know how to rebase from master before they submit a PR, so I doubt, highly, that they'll learn how to do a `git rebase -i` in order to make things cleaner. I've personally never seen a repo that utilizes all these "fancy" git commands (fancy to those developers) to make a clean history, I've always seen the messiest shit. Maybe that's sad for me in my career, though. I agree with you this is the best technical way, though, after reading some more comments in here, but not the best way for everyday developers.
- mikekchar 7y ago
  I think you are being down voted because people are confused about what you are saying. In git, a "branch" is just a tag on a particular commit. What we intuitively think of as a branch is the set of commits starting from where we diverged on master and ending up with the commit that is tagged with the branch name. There is nothing in git that actually stores that, though. When you rebase the branch, squashing commits, you create a new commit that is that sum of all the other commits. The other commits are left unreachable by any other branch and will be removed the next time you GC the database. What you are saying (I think) is to add a new branch called "merged-2020-03-03-BRANCHNAME" that points to the same commit as "BRANCHNAME" and then rebase BRANCHNAME, squashing the commits. In that way the old commits are still reachable and won't be removed. The downside, of course, is that it's a bit of a PITA if you want to find out what commit changed a line of code and why. You would narrow it down to "BRANCHNAME" and then you have to search for the archived version of it, switch to it and then continue looking at your code. But, in principle it could be done.

nickysielicki 7y ago

> Your commit history is what you will use later down the line (think 6 months, a year, 2 years later) to find a very nasty yet incredibly subtle bug in your application. No, it won’t. If I didn’t catch it then, I will have to read the code and understand it and spot it. At work, where my builds take anywhere from 30 minutes to 3 hours depending on the target, your git bisect on 1500 commits represents days of building and testing. Or I could just read the code again. Commit messages should provide context for going through the process of rereading the code, and little else. If a commit message literally describes what the code does (“change constant from X to Y”), it’s self-describing and unnecessary.

idoubtit 7y ago
It's okay if you really prefer to read a huge squashed commit instead of an atomic commit in a branch, but you made it sound like squashing was necessary in order to reduce building and testing. That's wrong. You can use `git bisect` on merge commits only, then, once the faulty merge is detected, bisect the commits of the merged branch. Bisecting merges won't take more time than bisecting squashed branches.

pbh101 7y ago

Agree with the thoughts on atomic and meaningful commits (though would drop the merge commit entirely), and I will rebase my commit history extensively at times to get it into a presentable state along the lines listed here. Bottom line: the history, content, and message of each commit, in addition to the resultant repo state, are all artifacts of your work and are worth shaping and crafting in order to optimize for communicating to yourself and your fellow developers in the future.

gouggoug 7y ago
> though would drop the merge commit entirely The merge commit is important though. It helps delimiting a set of commits that have meaning all together from the rest of your commit history. Here's an example: a -> b -> c -> d -> e -> f -> g -> h -> i -> j -> k In the history above, did you notice that commits c through h are actually related to one another? They were introduced a while ago by a feature branch. Compare the previous history to this: c -> d -> e -> f -> g -> h a -> b / \ i -> j -> k Now that's very clear that c through h are part of the same line of work. All those commits were probably part of 1 single pull request and were reviewed in the context of one another. My history here is still very linear. Yes, there's 1 little branching off, but that's all. At my previous company we had a codebase of well over 20k commits entirely made of those little branches branching off master, then merging back in. It made it for an incredibly readable history. An other added advantage of not removing the merge commit is that you can revert the entire merge of your branch in one go. Without the merge commit, you need to remove each commit one by one (and that's granted you remember which commit was the first one and which one was the last one).
- pbh101 7y ago
  Agreed your history is still linear. But I would suggest your merge commits are perhaps more trouble than they are worth. In commit sequences we prefix almost all commit subjects with the JIRA ticket ID, so the sequenced tickets are quite clearly marked as being related to each other without incurring the merge commit cost. As for reverting merge commits, I'd rather not personally have to remember the rules as to how to re-apply after the revert (revert the merge revert, then merge the branch again)[1], let alone working on spreading that knowledge losslessly across the organization. This is admittedly something I haven't done much of, but I'd rather revert ~3-9 individual commits, squash the reversion commits (usually with a bit more context on why we're reverting), and push. And it could be scripted. I haven't made a script for it as it doesn't happen that often, but between using rev-parse and a sed script in the GIT_EDITOR environment variable it would be pretty quick. Having merge commits also means that you have to teach everyone how to view diffs of merge commits, which admittedly isn't that hard, but it is still just one more barrier to know/teach/overcome, and the default behavior of showing a zero-line diff for a no-op merge commit doesn't help. [1] https://mirrors.edge.kernel.org/pub/software/scm/git/docs/howto/revert-a-faulty-merge.txt https://mirrors.edge.kernel.org/pub/software/scm/git/docs/ho...

bArray 7y ago

> Squashing that history of 25 commits, will create one giant > commit of 1500 changes. Have fun finding where in those > 1500 changes that little nasty bug was introduced! 100% agree, the only purpose of this is to make "pretty graphs" at the cost of destroying the history of the code changes. I prefer to write small commits with simple messages that explain exactly what that change does. Sometimes it costs some time in Git Gui selecting the lines that make up the commit, but ultimately it's worth it.

u801e 7y ago
> I prefer to write small commits with simple messages that explain exactly what that change does. One thing people should include in commit messages is why the change is being made. In other words, what's the purpose of the change. That can provide a lot of context that can be lost over time. So my commit messages include both what was done and why it was done. The why part can range from things like this method is needed for this feature that does something, or this fixes a bug that was causing this issue, or this other feature is no longer needed in context of this other reason, etc.
- mcv 7y ago
  What can also help is linking to the related Jira ticket (or whichever issue tracking system you use). And separate "cleanup" commits from commits that make meaningful changes. A commit that changes every line in a file because it's fixing incorrect indenting is fine. A commit that does that but also introduces a small functional change, makes that change invisible.
  - u801e 7y ago
    We've actually changed tracking systems over the years (meaning that the link in the commit message is no longer valid). So, it's best to include both the link and the actual text. > A commit that does that but also introduces a small functional change, makes that change invisible. While it's sometimes possible to separate the actual change from the formatting change by ignoring whitespace if the diff, it's definitely better to make each type of change in a separate commit.

Silhouette 7y ago

You have made a case for keeping the work-in-progress commits. The case the other way is that what should matter at the end of the feature development is the visible contribution it makes to the overall project, i.e., the finished code, tests, etc. This has two fundamental consequences. Firstly, in this model, your final commit that goes onto whatever you call your primary development branch should be a complete, self-contained piece of work, like adding a new feature or fixing some specific piece of broken behaviour. It should include any related tests, data files, etc. It should be something that can be reviewed in isolation and make sense. It should ideally be something that has minimal interactions with any other code that aren't strictly necessary, so it can be removed or replaced later if necessary with minimal effort to resolve or rewrite anything else that has happened since. Secondly, requiring all work-in-progress commits to also be meaningful and live forever in your shared repo significantly limits the usefulness of source control for the developer during the development stage. They might be experimenting with different approaches but want to keep track of a version where some specific aspect of the code is currently working. They might be about to refactor to clean some new code up but want to make sure they can't lose the exact version they had just got working due to unintended side effects. They don't want to be getting every individual commit code-reviewed or run any slow testing processes, just in case someone later comes along and cherry-picks that particular point in history to use as the basis for something else. So I will respectfully disagree with you that the full commit history at this level is as important as the final code. My reason for that is that the final code is complete, tested, reviewed, and going into the finished product, while anything in the work-in-progress commits that didn't survive to that point was determined, by the developer doing all of that work, at the time, with the aid of all of the testing and reviews and tools that were involved, not to be as valuable as the final version. If there really were interesting alternatives considered during development that might be useful to keep around for later reference, you can always tag or branch at that specific point and give it a meaningful name. However, I would suggest that this is like keeping issues open in your bug tracker when you know you'll never fix them: unless you also have some process for keeping track of such ideas and why they are valuable to refer back to them and take some useful action later, you're better off not having them clutter your main development process and tools forever.

gouggoug 7y ago

battery_cowboy 7y ago

But can't you just go back and reopen that branch? Just because you squash-merge the branch to master doesn't mean you can't keep the branch archived. I have seen where you basically just "rename" the branch (i.e.: make a new branch of the branch and just leave it as-is) to "merged-2020-03-03-BRANCHNAME" or something and then you don't touch it. Meanwhile, your master is a neat and tidy history of features added, tickets finished, and bugs fixed without all of the "fixed the thing I forgot to add" messages that inevitably sneak their way into your git log.

tremon 7y ago
without all of the "fixed the thing I forgot to add" messages This is why you use rebase --interactive or other ways of amending commits before submitting a PR. It is not an argument for commit squashing.
- battery_cowboy 7y ago
  > rebase --interactive like 90% of developers (maybe 95%) don't even know how to rebase from master before they submit a PR, so I doubt, highly, that they'll learn how to do a `git rebase -i` in order to make things cleaner. I've personally never seen a repo that utilizes all these "fancy" git commands (fancy to those developers) to make a clean history, I've always seen the messiest shit. Maybe that's sad for me in my career, though. I agree with you this is the best technical way, though, after reading some more comments in here, but not the best way for everyday developers.
mikekchar 7y ago
I think you are being down voted because people are confused about what you are saying. In git, a "branch" is just a tag on a particular commit. What we intuitively think of as a branch is the set of commits starting from where we diverged on master and ending up with the commit that is tagged with the branch name. There is nothing in git that actually stores that, though. When you rebase the branch, squashing commits, you create a new commit that is that sum of all the other commits. The other commits are left unreachable by any other branch and will be removed the next time you GC the database. What you are saying (I think) is to add a new branch called "merged-2020-03-03-BRANCHNAME" that points to the same commit as "BRANCHNAME" and then rebase BRANCHNAME, squashing the commits. In that way the old commits are still reachable and won't be removed. The downside, of course, is that it's a bit of a PITA if you want to find out what commit changed a line of code and why. You would narrow it down to "BRANCHNAME" and then you have to search for the archived version of it, switch to it and then continue looking at your code. But, in principle it could be done.