Squash your commits • Hacker News Board

Re: Squash your commits

#31

Post by klodolph » Fri, Apr 01, 2016, 7:18 PM UTC

ianleeclark wrote: post #3

This is a feature I've wanted for such a long time. While it's perfectly feasible to do through the command line, I've always found myself having to force push to update the history on github's side of things. I typically did it through git rebase -i HEAD~N, so maybe someone here on HN knows of a better way to squash a commit whenever you're updating remote history. Albeit, it seems that updating remote history with…

Two things you can do differently, if you feel this would be appropriate.

Instead of git rebase -i HEAD~N, I typically use git reset --soft HEAD~N followed by git commit.

Instead of git push --force, use git push --force-with-lease. This only updates the remote if the remote's current state is what you expect. For example, it will fail if someone else has pushed to the same branch in the meantime.

Re: Squash your commits

#32

Post by dsmithatx » Fri, Apr 01, 2016, 7:19 PM UTC

natrius wrote: post #25

cballard wrote: post #10

This is a bad idea masquerading as a good idea. Before making a pull request (or doing any sort of merge), you should rebase against upstream master (or whatever you're going to push to). However, keeping distinct atomic commits that change one and only one small thing, when possible, is much preferable if bisect or blame is used. If you have broken or poorly written commits, use fixup, reword, squash, etc. in rebase…

I used to strongly believe what you do until my company started using Phabricator, which forces the squash workflow on you. It makes your history more useful, not less. The pull request is the appropriate unit of change for software. Make small commits as you develop, then squash them down into a single meaningful change to the behavior of your software.

As a git novice I wonder, doesn't a proper workflow do the same thing? When I submit a feature branch it might have a lot of ugly commits. However, once I merge it to an integration branch there is one nice commit explaining what I did.

When coworkers create Pull requests I don't go through all of their commits and changes along the way. I just look at the diff so, I don't see the need for them to squash it first.

Re: Squash your commits

#33

Post by glhaynes » Fri, Apr 01, 2016, 7:20 PM UTC

natrius wrote: post #25

cballard wrote: post #10

This is a bad idea masquerading as a good idea. Before making a pull request (or doing any sort of merge), you should rebase against upstream master (or whatever you're going to push to). However, keeping distinct atomic commits that change one and only one small thing, when possible, is much preferable if bisect or blame is used. If you have broken or poorly written commits, use fixup, reword, squash, etc. in rebase…

I used to strongly believe what you do until my company started using Phabricator, which forces the squash workflow on you. It makes your history more useful, not less. The pull request is the appropriate unit of change for software. Make small commits as you develop, then squash them down into a single meaningful change to the behavior of your software.

It seems like it'd be nice to have two levels of granularity exposed in views of a source control system's history, basically corresponding to pull requests and commits. So you could drill-down to individual commits as needed, but would normally be able to work at the PR level.

Re: Squash your commits

#34

Post by LegNeato » Fri, Apr 01, 2016, 7:20 PM UTC

Awesome! Phabricator got me addicted to the clean history of squashing commits and the logical changes staying together in source control (as opposed to grouping in a PR).

Re: Squash your commits

#35

Post by dsmithatx » Fri, Apr 01, 2016, 7:21 PM UTC

jecjec wrote: post #23

Git is actually unusable software. Fucking around with rebase WILL cause you more headaches than this is worth. The number of downvotes this has received tells me just how deluded the alleged community of software writers actually is.

Strange that everyone these days uses Git considering it's unusable.

Re: Squash your commits

#36

Post by jakub_g » Fri, Apr 01, 2016, 7:22 PM UTC

haberman wrote: post #22

cballard wrote: post #10

This is a bad idea masquerading as a good idea. Before making a pull request (or doing any sort of merge), you should rebase against upstream master (or whatever you're going to push to). However, keeping distinct atomic commits that change one and only one small thing, when possible, is much preferable if bisect or blame is used. If you have broken or poorly written commits, use fixup, reword, squash, etc. in rebase…

There's no guarantee that every individual commit of a feature branch is meaningful, or even builds. It also makes the history of the master branch a lot harder to read when it has tons of commits representing the minutiae of the feature's development.

It really depends on each individual's workflow. I tend to use lots of "in progress" commits (each time things are "green"), and as I go, I regularly squash the commits, so in the final pull request I typically have several commits (and if I wasn't squashing it would have been a dozen). If I do feature and a refactor, they are always separate commits, it's easier to review these and bisect if something turns out to go wrong.

Some people might do similar things but they might not assure each commit is green, and they never squash anything (so you end up with non-meaningful commits).

As @3JPLW said, I see when it can be useful for opensource maintainers to have the option to squash someone's commits, when the change is small, but there are many commits (due to a review ping-pong etc)

Re: Squash your commits

#37

Post by Bahamut » Fri, Apr 01, 2016, 7:22 PM UTC

3JPLW wrote: post #21

cballard wrote: post #10

This is a bad idea masquerading as a good idea. Before making a pull request (or doing any sort of merge), you should rebase against upstream master (or whatever you're going to push to). However, keeping distinct atomic commits that change one and only one small thing, when possible, is much preferable if bisect or blame is used. If you have broken or poorly written commits, use fixup, reword, squash, etc. in rebase…

Sure, it's a terrible feature to always use. And it's likely to be of little use to contributors who know how to use Git well. But in large open-source projects, often new contributors make a small change that needs a few minor corrections. Eliminating that final back-and-forth ("squash please") is a huge win for maintainers.

Not only maintainers - anyone tracing back through the history to find out what broke their use case.

Re: Squash your commits

#38

Post by falsedan » Fri, Apr 01, 2016, 7:22 PM UTC

cballard wrote: post #10

This is a bad idea masquerading as a good idea. Before making a pull request (or doing any sort of merge), you should rebase against upstream master (or whatever you're going to push to). However, keeping distinct atomic commits that change one and only one small thing, when possible, is much preferable if bisect or blame is used. If you have broken or poorly written commits, use fixup, reword, squash, etc. in rebase…

I use this workflow:

  1. branch off master
  2. work, commit, push, test (on CI server)
  3. decide it's time to ship
  4. rebase -i, push, test (again)
  5. git checkout master && git merge --no-ff feature_branch

(make the merge commit message a summary of the feature)

master ends up being a list of feature branch commits, bookended by the merge commit which introduced the feature. Getting the squash commit diff is as easy as 'git diff feature_branch_merge^..feature_branch_merge'.

Re: Squash your commits

#39

Post by pkamb » Fri, Apr 01, 2016, 7:23 PM UTC

I'm waiting for the code review tool that takes a massive squashed PR and cuts it up into a series of clean, atomic commits that can be reviewed individually.

Re: Squash your commits

#40

Post by scrollaway » Fri, Apr 01, 2016, 7:24 PM UTC

natrius wrote: post #25

cballard wrote: post #10

This is a bad idea masquerading as a good idea. Before making a pull request (or doing any sort of merge), you should rebase against upstream master (or whatever you're going to push to). However, keeping distinct atomic commits that change one and only one small thing, when possible, is much preferable if bisect or blame is used. If you have broken or poorly written commits, use fixup, reword, squash, etc. in rebase…

I used to strongly believe what you do until my company started using Phabricator, which forces the squash workflow on you. It makes your history more useful, not less. The pull request is the appropriate unit of change for software. Make small commits as you develop, then squash them down into a single meaningful change to the behavior of your software.

Sometimes yes, sometimes no. Merge commits are nasty imo and I'm glad we can now forbid them outright, but a full squash isn't always the solution either.

Take a look at this PR for example:

https://github.com/HearthSim/python-unitypack/pull/4

Lots of back & forth. All the commits are related, and the PR is there to land all of those commits at once. I could land some of them right now (as they're safe to land), but keeping them in the PR keeps everything related in the same place (and none of them are required until that last commit lands).

A PR mirrors a "patchset" on mailing lists. You don't always want to squash all of it.

What you do want to avoid is a situation like this:

https://github.com/jleclanche/django-push-notifications/pull...

Where the original author creates their original commit and doesn't know how to --amend + push --force to the PR, and you end up with a ton of commits which you don't want to land all at once.