4 ms·

> the history is littered with utter garbage commits like the following: "commit this before I get on the plane" "whoops, make this compile" "WTF?" You have a

> the history is littered with utter garbage commits like the following: "commit this before I get on the plane" "whoops, make this compile" "WTF?" You have a problem with commits here, not the lack of squashing. A messy history is only messy if you make it so. > These add no benefit to history, and actually provide an impediment to bisecting (since a lot of these intermediate revisions will not even compile). This is also a problem---every commit should compile, even if only for precisely the reason of bisecting. Squashing commits is avoiding the problem; instead, perhaps educate the others on proper practices for committing, and enforce those practices.

tenken 10y ago
Ya I do an end of the day commit of alot of things (to start fresh in the morning) it doesnt mean commit trash at the end of the day. It implies to stop 5 minutes early, make a clean commit of the WIP (which compiles, or throws no syntax errors in the PHP world) and call it day.
- mikegerwitz 10y ago
  To aid in comprehension, fine-grained bisecting, and debugging, I commit as small of a patch as I can for a given change; if I have to use "and" in the commit message, I might have gone too far. I also commit a ChangeLog-style message describing the changes so that they can be eyeballed without having to read through the diff. I find that especially important for bisecting, and on that merit alone (I have other reasons as well) reject squashing commits---it makes bisecting useless for large changes. When someone commits what you can better call a project, you're going to struggle, even if it's easy to comprehend the code. Same goes for review requests---we encourage our team to post small reviews, and all of us get frustrated when we have to try to grok a 500+ line diff (unless those lines are entire new or entirely removed large hunks). At the end of the day, my WIP is usually quite small, with some exceptions. Even on complicated changes---much of that planning is on paper (or mental paper) or writing test cases. WIP can be hard to get back into when flow is broken, letalone the next day, or after the weekend.
- cortesoft 10y ago
  Depending on what you are working on, this sort of "cleanup in 5 minutes" task might be impossible. There are lots of reasons you might need to go a longer time between working code than the length of time you want to go with no backup or save point.
developer2 10y ago
The nice thing about git is that your commits can be garbage. Until you push (or, much less commonly on most teams, let someone pull from you), your local commits can be an utter disaster, and it just does not matter. From a purely technical standpoint, there is no excuse for messy logs. Any mess is the result of the user, not the software. The problem is that using rebase to clean up your local commit log before sharing with others isn't the easiest thing to learn. Even once you supposedly "know what you are doing", it is still possible to make a mistake. Git has a lot of amazing functionality, but the majority of commands are not intuitive to use out of the box. Even GUI frontends to git don't manage to simplify the more complex commands all that much. I'd like to believe I'm an "intermediate" git user, but I run into issues often enough that I'm sure I overestimate my knowledge. Most teams I've seen using git wind up using only the core commands (clone, commit, fetch/pull, merge, push), essentially using git as a drop-in replacement for svn without taking advantage of the additional possibilities git offers. Again, this is because becoming a git guru is a steep learning curve. It doesn't matter if 19 people on a project know everything about git; it only takes a single 20th person to make a tangled mess out of a centrally shared repository. I've spent many an afternoon working to rectify botched rebases and similar issues. One example is forcing a push for a specific branch, knowing that the resulting destruction of history is desired and not harmful to others; only to forget to specify the branch name on command line, which results in force pushing all branches. Whoops!
- mikegerwitz 10y ago
  > The nice thing about git is that your commits can be garbage. That's a dangerous practice. When you first commit the changes, you have all the necessary context, and hopefully flow hasn't been interrupted. If you rebase hours, days, maybe weeks later, then that context is completely lost---it's just like trying to get back into a project that amount of time later. The reason the detail you'd put into your commits is so useful is because you have a perspective that others won't; if you return to it later, you'll be reading the diff and suffering just like others, albeit with a bit more knowledge.
  - developer2 10y ago
    I could have been more specific about the scenario I was referring to. Many people commit to git very frequently in such a way that their local git history is more like an IDE's undo log. You can commit 10+ times in the span of an hour, using the log as nothing more than a savepoint system. This is really common from people who for some unfathomable reason perform a commit before testing, which results in a lot of those one-liner commits that really do not deserve their own commit (this specific task can be done via amend, but you don't see it from most devs). Git commits are so fantastically cheap, and especially easy if it's a one second task via keyboard shortcut in your IDE rather than terminal commands. You wind up with a lot of commits throughout the day all related to accomplishing a simple task, and then rebase them down into a smaller number of meaningful chunks of work. I agree that this should not be the workflow for larger units of work. If you're working on a week-long task, you would use this flow to refactor each day's 50 commits down to what you would have normally committed (maybe 1-5 commits for the day). This allows your feature branch's log to contain meaningful information. At the end of the week, when your feature branch is complete, you then have the additional choice as to whether the branch's already-more-compact history is meaningful; if not, you can further squash the merge commit. tldr: >> When you first commit the changes, you have all the necessary context Rebasing your local changes before sharing is for the case where you're spamming loads of meaningless commits that don't have - or deserve - their own context.
prodigal_erik 10y ago
This. If your team writes bad comments and cryptic names, you don't respond by deleting all comments and obfuscating all names, you tell them to do better work. Why are commits taken less seriously?
jasode 10y ago
>--every commit should compile, No, that advice is wrong and not relevant to a git private branch. That strict & disciplined attitude about commits was relevant for older centralized source control tools with lock/checkout/checkin/unlock such as CVS and SVN. In that previous scenario, your colleagues depended on the shared repo to properly compile and therefore, you shouldn't "break the build" and derail the team. >educate the others on proper practices for committing, On a private branch, people should commit whenever they want on any whim of a reason. This will result in many commits that don't compile/build. That's ok. That's what the later step of "squash" into "logical" commits is for. To repeat a previous comment about it: The confusion is that the same "git commit" command is used for 2 very different semantic purposes: (1) git commit -m "fixed bug #23984" --> as Logical-Unit-Work and worthy of bisect (2) git commit -m "wip" --> as meaningless backup/savepoint like Ctrl+S save The type (2) was for the programmer's internal purposes of safety backups, cleaning up whitespace, typos in comments, reflexive muscle memory of saving often, etc. Type (2) commits can have deliberate broken syntax and they're not meant to be built or be bisected. Type (2) commits should never be discouraged because saving work often (including broken midstream work) is a good habit but from an outsiders perspective of the reviewers upstream, they are way too noisy. The spurious commits could be less than 30 seconds apart with no compile/build step in between. >Squashing commits is avoiding the problem I hope it's now clear that "squashing" is the correct tool for Type 2 commits.
- mikegerwitz 10y ago
  > On a private branch, people should commit whenever they want on any whim of a reason. The concern was when that private branch isn't cleaned up before it is make public; it then becomes an issue. Though see my reply to developer2 (sibling of your post) for rationale against garbage commits to begin with.
- harperlee 10y ago
  But if I commit on a whim, e.g. before jumping on the plane, how can I then patch to separate some changes in two commits?
  - jon-wood 10y ago
cortesoft 10y ago
I commit for lots of reasons, though. Sometimes, I am about to try something crazy with the code, and I want a save point to go back to. Sometimes, I want to switch computers in the middle of working on something (going from desktop to laptop, for example), so I commit and push to the other computer. Also, if I am working on something very complex, I might go hours or even days with code that can't compile. I don't want to risk losing that because I was avoiding committing. A lot of these problems can be resolved with the proper use of --fixup and --squash and auto squash rebasing.
- mikegerwitz 10y ago
  My argument was for not retaining those commits, or not committing them in the first place; drewg123 was complaining about the situation from others, meaning that their private branches were not cleaned up prior to pushing. You wouldn't push your WIP branch, I would assume. I have also pushed for the purpose of pulling on another PC; that commit then gets `reset --soft`'d.
  - cortesoft 10y ago
    I usually want to push it somewhere, to maintain a backup in case of hardware failure.
stepanhruda 10y ago
It'd pe perfect, if only we could take people out of the equation. Seriously, I think that the cost/benefit ratio of a perfect history is so terrible that there are very very few people that follow through with it. If one submits a PR and it has a messy history, what is the reviewer supposed to do? Make them go through the commits and clean them up? How will the reviewer verify that every commit compiles? Make them squash it? We are back to square one. If you only have squash commits from PRs and run CI on them, you are sure that every commit compiles.
- mikegerwitz 10y ago
  > If one submits a PR and it has a messy history, what is the reviewer supposed to do? Make them go through the commits and clean them up? That's up to the project. Some do, yes, for the reasons that I described---it's much easier to review a series of patches that can be comprehended, and has benefits later on (e.g. bisecting). If the patch is large from a contributor, and a bisect arrives at that commit, and the contributor is not the one doing the debugging, it can be a frustrating and inefficient experience. I've wasted many nights on something that could have otherwise been immediately obvious. > If you only have squash commits from PRs and run CI on them, you are sure that every commit compiles. It's not difficult to loop through commits and make sure they compile. In fact, bisect can do it for you, if you aren't fond of command-line loops.
  - stepanhruda 10y ago
    For larger projects this takes literally hours
    - mikegerwitz 10y ago
      It's not a one-size-fits-all type of thing; if it doesn't work for your project, then you need to try something else. Where I work, we don't enforce that each commit build---it's known to be a good practice, and they're going to have someone flip out on them if they're trying to debug something and they have to skip a dozen commits when bisecting (incidentally, I had that problem today). In that case, it's a cultural thing. If someone consistently commits code that doesn't build, then they should be addressed. If it's a random contributor to a project, more care should probably be taken. If building each commit isn't feasible, then maybe only building modified files (that is, a normal `make` for example rather than a fresh workspace) is better than nothing. If that's too long, maybe build a few sample commits. Etc. In any case, even if you don't build each commit, the history is still useful.

tenken 10y ago

Ya I do an end of the day commit of alot of things (to start fresh in the morning) it doesnt mean commit trash at the end of the day. It implies to stop 5 minutes early, make a clean commit of the WIP (which compiles, or throws no syntax errors in the PHP world) and call it day.

mikegerwitz 10y ago
To aid in comprehension, fine-grained bisecting, and debugging, I commit as small of a patch as I can for a given change; if I have to use "and" in the commit message, I might have gone too far. I also commit a ChangeLog-style message describing the changes so that they can be eyeballed without having to read through the diff. I find that especially important for bisecting, and on that merit alone (I have other reasons as well) reject squashing commits---it makes bisecting useless for large changes. When someone commits what you can better call a project, you're going to struggle, even if it's easy to comprehend the code. Same goes for review requests---we encourage our team to post small reviews, and all of us get frustrated when we have to try to grok a 500+ line diff (unless those lines are entire new or entirely removed large hunks). At the end of the day, my WIP is usually quite small, with some exceptions. Even on complicated changes---much of that planning is on paper (or mental paper) or writing test cases. WIP can be hard to get back into when flow is broken, letalone the next day, or after the weekend.
cortesoft 10y ago
Depending on what you are working on, this sort of "cleanup in 5 minutes" task might be impossible. There are lots of reasons you might need to go a longer time between working code than the length of time you want to go with no backup or save point.

developer2 10y ago

The nice thing about git is that your commits can be garbage. Until you push (or, much less commonly on most teams, let someone pull from you), your local commits can be an utter disaster, and it just does not matter. From a purely technical standpoint, there is no excuse for messy logs. Any mess is the result of the user, not the software. The problem is that using rebase to clean up your local commit log before sharing with others isn't the easiest thing to learn. Even once you supposedly "know what you are doing", it is still possible to make a mistake. Git has a lot of amazing functionality, but the majority of commands are not intuitive to use out of the box. Even GUI frontends to git don't manage to simplify the more complex commands all that much. I'd like to believe I'm an "intermediate" git user, but I run into issues often enough that I'm sure I overestimate my knowledge. Most teams I've seen using git wind up using only the core commands (clone, commit, fetch/pull, merge, push), essentially using git as a drop-in replacement for svn without taking advantage of the additional possibilities git offers. Again, this is because becoming a git guru is a steep learning curve. It doesn't matter if 19 people on a project know everything about git; it only takes a single 20th person to make a tangled mess out of a centrally shared repository. I've spent many an afternoon working to rectify botched rebases and similar issues. One example is forcing a push for a specific branch, knowing that the resulting destruction of history is desired and not harmful to others; only to forget to specify the branch name on command line, which results in force pushing all branches. Whoops!

mikegerwitz 10y ago
> The nice thing about git is that your commits can be garbage. That's a dangerous practice. When you first commit the changes, you have all the necessary context, and hopefully flow hasn't been interrupted. If you rebase hours, days, maybe weeks later, then that context is completely lost---it's just like trying to get back into a project that amount of time later. The reason the detail you'd put into your commits is so useful is because you have a perspective that others won't; if you return to it later, you'll be reading the diff and suffering just like others, albeit with a bit more knowledge.
- developer2 10y ago
  I could have been more specific about the scenario I was referring to. Many people commit to git very frequently in such a way that their local git history is more like an IDE's undo log. You can commit 10+ times in the span of an hour, using the log as nothing more than a savepoint system. This is really common from people who for some unfathomable reason perform a commit before testing, which results in a lot of those one-liner commits that really do not deserve their own commit (this specific task can be done via amend, but you don't see it from most devs). Git commits are so fantastically cheap, and especially easy if it's a one second task via keyboard shortcut in your IDE rather than terminal commands. You wind up with a lot of commits throughout the day all related to accomplishing a simple task, and then rebase them down into a smaller number of meaningful chunks of work. I agree that this should not be the workflow for larger units of work. If you're working on a week-long task, you would use this flow to refactor each day's 50 commits down to what you would have normally committed (maybe 1-5 commits for the day). This allows your feature branch's log to contain meaningful information. At the end of the week, when your feature branch is complete, you then have the additional choice as to whether the branch's already-more-compact history is meaningful; if not, you can further squash the merge commit. tldr: >> When you first commit the changes, you have all the necessary context Rebasing your local changes before sharing is for the case where you're spamming loads of meaningless commits that don't have - or deserve - their own context.

prodigal_erik 10y ago

This. If your team writes bad comments and cryptic names, you don't respond by deleting all comments and obfuscating all names, you tell them to do better work. Why are commits taken less seriously?

jasode 10y ago

>--every commit should compile, No, that advice is wrong and not relevant to a git private branch. That strict & disciplined attitude about commits was relevant for older centralized source control tools with lock/checkout/checkin/unlock such as CVS and SVN. In that previous scenario, your colleagues depended on the shared repo to properly compile and therefore, you shouldn't "break the build" and derail the team. >educate the others on proper practices for committing, On a private branch, people should commit whenever they want on any whim of a reason. This will result in many commits that don't compile/build. That's ok. That's what the later step of "squash" into "logical" commits is for. To repeat a previous comment about it: The confusion is that the same "git commit" command is used for 2 very different semantic purposes: (1) git commit -m "fixed bug #23984" --> as Logical-Unit-Work and worthy of bisect (2) git commit -m "wip" --> as meaningless backup/savepoint like Ctrl+S save The type (2) was for the programmer's internal purposes of safety backups, cleaning up whitespace, typos in comments, reflexive muscle memory of saving often, etc. Type (2) commits can have deliberate broken syntax and they're not meant to be built or be bisected. Type (2) commits should never be discouraged because saving work often (including broken midstream work) is a good habit but from an outsiders perspective of the reviewers upstream, they are way too noisy. The spurious commits could be less than 30 seconds apart with no compile/build step in between. >Squashing commits is avoiding the problem I hope it's now clear that "squashing" is the correct tool for Type 2 commits.

mikegerwitz 10y ago
> On a private branch, people should commit whenever they want on any whim of a reason. The concern was when that private branch isn't cleaned up before it is make public; it then becomes an issue. Though see my reply to developer2 (sibling of your post) for rationale against garbage commits to begin with.
harperlee 10y ago
But if I commit on a whim, e.g. before jumping on the plane, how can I then patch to separate some changes in two commits?
- jon-wood 10y ago

cortesoft 10y ago

I commit for lots of reasons, though. Sometimes, I am about to try something crazy with the code, and I want a save point to go back to. Sometimes, I want to switch computers in the middle of working on something (going from desktop to laptop, for example), so I commit and push to the other computer. Also, if I am working on something very complex, I might go hours or even days with code that can't compile. I don't want to risk losing that because I was avoiding committing. A lot of these problems can be resolved with the proper use of --fixup and --squash and auto squash rebasing.

mikegerwitz 10y ago
My argument was for not retaining those commits, or not committing them in the first place; drewg123 was complaining about the situation from others, meaning that their private branches were not cleaned up prior to pushing. You wouldn't push your WIP branch, I would assume. I have also pushed for the purpose of pulling on another PC; that commit then gets `reset --soft`'d.
- cortesoft 10y ago
  I usually want to push it somewhere, to maintain a backup in case of hardware failure.

stepanhruda 10y ago

It'd pe perfect, if only we could take people out of the equation. Seriously, I think that the cost/benefit ratio of a perfect history is so terrible that there are very very few people that follow through with it. If one submits a PR and it has a messy history, what is the reviewer supposed to do? Make them go through the commits and clean them up? How will the reviewer verify that every commit compiles? Make them squash it? We are back to square one. If you only have squash commits from PRs and run CI on them, you are sure that every commit compiles.

mikegerwitz 10y ago
> If one submits a PR and it has a messy history, what is the reviewer supposed to do? Make them go through the commits and clean them up? That's up to the project. Some do, yes, for the reasons that I described---it's much easier to review a series of patches that can be comprehended, and has benefits later on (e.g. bisecting). If the patch is large from a contributor, and a bisect arrives at that commit, and the contributor is not the one doing the debugging, it can be a frustrating and inefficient experience. I've wasted many nights on something that could have otherwise been immediately obvious. > If you only have squash commits from PRs and run CI on them, you are sure that every commit compiles. It's not difficult to loop through commits and make sure they compile. In fact, bisect can do it for you, if you aren't fond of command-line loops.
- stepanhruda 10y ago
  For larger projects this takes literally hours
  - mikegerwitz 10y ago
    It's not a one-size-fits-all type of thing; if it doesn't work for your project, then you need to try something else. Where I work, we don't enforce that each commit build---it's known to be a good practice, and they're going to have someone flip out on them if they're trying to debug something and they have to skip a dozen commits when bisecting (incidentally, I had that problem today). In that case, it's a cultural thing. If someone consistently commits code that doesn't build, then they should be addressed. If it's a random contributor to a project, more care should probably be taken. If building each commit isn't feasible, then maybe only building modified files (that is, a normal `make` for example rather than a fresh workspace) is better than nothing. If that's too long, maybe build a few sample commits. Etc. In any case, even if you don't build each commit, the history is still useful.