Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Squash your commits

github.com

Post reply on HN
Print view View original

51–60 of 350 posts

Re: Squash your commits

#51

Post by tunesmith »

BinaryIdiot wrote: post #45
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…

> 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) See, and maybe this is because I'm just dumb or something, but I have never gotten rebasing to work for me. Ever. Every single time I do it I read at east 3 articles about it so I don't screw something up, I attempt to do it and ultimately I lose a bunch of work. I just don't ge…

I think the advice to rebase runs up against the business pattern of pushing your branch as soon as you create it (git-flow and a lot of jira/stash integrations work like this). Also some teams want to see evidence of your commits as you make them, which means pushing as you commit.

If you have a branch and it's already pushed, rebasing just feels kind of funny and can sometimes cause a lot of problems if anyone else has checked it out.

If you have a branch and it's local only, then merging from mainline into your branch and selecting rebase instead of merge is relatively painless.

Re: Squash your commits

#53

Post by natrius »

scrollaway wrote: post #40
natrius wrote: post #25

Earlier quoted context omitted.

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…

To clarify, pull requests shouldn't be squashed before review. They should be squashed when they get merged in, like this GitHub feature does. The granular commit history is useful during code review. It is not useful as a future developer trying to evaluate how the behavior of the program changed over time.

Re: Squash your commits

#54

Post by lomnakkus »

cballard wrote: post #18
michaelmior wrote: post #17

Earlier quoted context omitted.

I used to feel the same way (and still do to some degree). However I think the issue is more nuanced. I agree that rebasing beforehand is a good idea. But I can see the value in keeping commits on the master branch corresponding to specific features or bug fixes (which presumably map to PRs). I think the argument can be made that if you don't feel comfortable performing a squashed merge of a PR, then that PR contains…

Small PRs are an issue because PRs are dependent on other users and can't be dependent on a prior PR. Let's say we're adding an interface/typeclass/protocol and a concrete implementation. I'd say these should be two separate commits, as they're adding two different things. An interface doesn't require a provided implementation to work. But, if we were to create those as two separate pull requests, it would be more wo…

> and can't be dependent on a prior PR.

This pinpoints the major problem exactly. Without dependencies between PRs there's really no sane way (with this feature enabled) to submit a series of commits while expecting those commits to remain separate.

Oh, and I object to the general sentiment in the responses to your post that seem to value drive-by/inexperienced contributors over the "experts". Yes, we definitely should make things as easy/simple as possible for new contributors, but NOT at the expense of adding a gotcha for expert contributors. The experts are what keep a project going over many years instead of just releasing version upon version of trivial spelling fixes.

(And, btw, the default "merge" option for GitHub PRs also sucks. It should be possible to simply disallow non-FF merges and to force all merges to be FF. EDIT: Interestingly, this seems to be about the only workflow explicitly forbidden by the new rules... unless, of course, you're willing to merge everything manually using your local copy of the repo and pushing from that to GH.)

Re: Squash your commits

#55

Post by pkamb »

After completing a feature (or part of one) I often "squash" manually by performing a mixed reset from the tip of the in-development branch back to a good parent commit.

This leaves all of the feature's changes in your working copy, which you can then stage by line/hunk and individually commit in clean, atomic pieces with the benefit of already having written the final code.

This removes the in-progress commits like a Squash, but pieces of code can still be brought in as individual easy-to-review commits. And it's potentially easier to understand / perform manually than rewriting history via a git command that would do the same thing.

Re: Squash your commits

#56

Post by scrollaway »

natrius wrote: post #53
scrollaway wrote: post #40

Earlier quoted context omitted.

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…

To clarify, pull requests shouldn't be squashed before review. They should be squashed when they get merged in, like this GitHub feature does. The granular commit history is useful during code review. It is not useful as a future developer trying to evaluate how the behavior of the program changed over time.

Right, sometimes. Pull requests are not necessarily a "unit of change" like you mentioned, though. For example, the first link I gave should not be squashed. But I don't want it to create a merge commit either.

I'm a little underwhelmed with the feature, it looks like it's either "squash everything" or "make a merge commit". There's no option to rebase & merge and/or selectively squash.

Re: Squash your commits

#57

Post by gonyea »

I hate squashing branches. There's a lot of value in commit messages; they're educational and are a form of documentation. It's on the developer to squash the "oops" commits, during a rebase, rewriting the commit message so it has some value when going back in time to look at changes.

I'd love to see a commit linter, that points at commits with text like "oops" and "fix my derp" to suggest possible commits to squash.

Git history shouldn't resemble a hot mess, but the evolution of code should be pretty granular. I'd take the hot mess over full squashing, though.

Re: Squash your commits

#58

Post by joshuahutt »

BinaryIdiot wrote: post #45
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…

> 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) See, and maybe this is because I'm just dumb or something, but I have never gotten rebasing to work for me. Ever. Every single time I do it I read at east 3 articles about it so I don't screw something up, I attempt to do it and ultimately I lose a bunch of work. I just don't ge…

  $ git checkout master
  $ git pull
  $ git checkout branch-name
  $ git rebase master
If there are merge conflicts, open the affected file(s) and resolve them. Then:

  $ git add filename.ext
  $ git rebase --continue
Finally:

  $ git push origin branch-name
If you've already pushed the branch, use -f. Make sure to always specify the branch name when using that flag!

Re: Squash your commits

#59

Post by WorldMaker »

Sometimes I feel like it's a minority position, but I think it strange all the efforts people go to in order to essentially make the git DAG look like a (lie of a) straight-line CVS or SVN commit list. Seeing how the sausage was actually made (no rebases, no squashes, sometimes not even fast-forwards) isn't pretty, but it is meaningful and will tell you a great deal about a project and its developers... I trust that. It's real and visceral and how software is actually made and you can learn from that or find things to explore in that jungle. Projects with multiple developers that yet have straight line commit histories and super tidy commits are aberrations and full of little lies...

Kudos to GitHub for providing this feature that a lot of people have asked for. I obviously don't plan to use it, but I appreciate that it's an option for those people that like their small, harmless lies. ;)

Re: Squash your commits

#60

Post by golergka »

This, as rebase, is a great idea for people who would rather use SVN instead, but have to use git for some reason or another.

I mean, if you want your history simplified into a linear squashed series of commits, why the hell do you use VCS which models a history as acyclic directional graph in the first place?

Post reply on HN