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

61–70 of 350 posts

Re: Squash your commits

#61

Post by echion »

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.

There's no guarantee, but there are many benefits to striving for this ("git bisect run", CI test results).

Re: Squash your commits

#62

Post by trollingineeer »

joshmanders wrote: post #6
spleeyah wrote: post #2

Unsure if April Fool's joke...

Would be the worst April Fools joke ever. Even above "drop mic" by Google.

Drop mic wasn't bad. People are just stupid. And you're a sheep for just going along with the "it's the worst prank" train. Start thinking for yourself Joshie.

Re: Squash your commits

#63

Post by azernik »

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…

A few tips!

1. Always use the "upstream" branch as your rebase target - "git rebase -i master", or " git rebase -i origin/master". This is almost always what you want, and picking the wrong base is the most common error I've seen when teaching people rebase -i

2. Use autosquash! https://robots.thoughtbot.com/autosquashing-git-commits. If you have trouble with the text-editor interface you get when you run rebase -i, this will both handle its usage, and in the long run give you some visual examples of how the interface is supposed to be used. If you're really into this, set the config option "rebase.autoSquash true" to avoid the extra command-line flag.

3. If you mess up and realize in the middle, git rebase --abort.

4. Use the reflog after the fact for both finding and undoing mistakes: git diff branchname branchname@{1} to check for unintended code differences, and git reset --hard branchname@{1} to undo the rebase.

Re: Squash your commits

#64

Post by brown9-2 »

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…

Use `rebase --interactive` so you can have a better idea of what is going on.

Re: Squash your commits

#65

Post by golergka »

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…

Why would you want to rewrite your whole work history and change the actual state of the repository at each of your commits? Why don't just merge?

Re: Squash your commits

#66

Post by devit »

The key principle is that the software should always correctly work at any point in the chain of commits, so you must squash commits that are "oops, fix X in previous commit".

Once that is satisfied, commits should be as small as possible, so that information about the grouping of changes is preserved.

Re: Squash your commits

#67

Post by kylnew »

WorldMaker wrote: post #59

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.…

Have you ever tried following a change in a repo that came from an unsquashed PR? It's hell.

What's truly meaningful IMO is a git log that reads like a product change log

Re: Squash your commits

#68

Post by hkdobrev »

dlubarov wrote: post #47
lotyrin wrote: post #16

Any plans to allow for squashing but with a merge instead of a fast-forward or fast-forwards without squashing? CI will still run against the hypothetical merge commit, no? I wonder if there are edge cases where merge vs squash+fast-forward would result in different conflict resolutions and different trees, so master could end up with a tree that didn't have tests run against it.

> master could end up with a tree that didn't have tests run against it Isn't that already the norm? I.e., most projects run CI on an unmerged PR, then merge to master, then run CI on master to see if the merged tree is actually good. Are there projects which test a PR's post-merge tree before updating master? That would be nice, but I don't know of any tools which support that workflow, and it wouldn't scale forever…

Travis CI for example and I'd imagine most CI servers run tests on the `MERGE_HEAD` of the PR which like lotyrin said is the hypothetical merge commit. If a PR is opened from latest master, CI runs on `MERGE_HEAD` which is the same with the merge commit after the PR is merged. But if something was pushed to master after the feature branch was branched off then it's not the same commit. That's why it's recommended to run CI after merging again. But by default tests are not run on the PR HEAD which is the tip of the feature branch.

Re: Squash your commits

#69

Post by phasmantistes »

WorldMaker wrote: post #59

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.…

I actually disagree. Large teams that still have linear commit histories doesn't mean it is a lie. It means that the code review process is more important that the code writing process.

For example: I check out a repository, and create a local feature branch. I create a commit containing the tests for the new feature, then one for the first draft of the new feature, then two or three for bugfixes. Each commit is small, and self-contained, but importantly isn't standalone. If someone checked out the repository in the middle of my chain of commits, they wouldn't have a working product. Then I upload my change for code review. There's no point in reviewing each of my ~5 commits individually: they only make sense to the reviewer as a combined unit. And there's no point in landing them individually: they only make sense for the overall project history as a combined unit.

In a project with many developers (e.g. 1,000 like the Chromium project), every developer has different local practices. Some keep their work based on HEAD of master via rebase, others via merges. Some do test-driven development, some don't. Making the code review the atomic unit of work, rather than the messy string of local commits, helps the project enforce common etiquette, commit formatting, and readable history.

Re: Squash your commits

#70

Post by JoshTriplett »

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…

If someone prepares a pull request with a well-structured series of commits, making a logical series of changes, where the project builds and passes tests after each commit, then those commits shouldn't get squashed.

However, I frequently see people adding more commits on top of a pull request to fix typos, or do incremental development, where only the final result builds and passes, but not the intermediate stages, and where the changes are scattered among the commits with no logical grouping. In that case, I'd rather see them squashed and merged than merged in their existing form, and having a button to do that makes it more likely to happen.

Post reply on HN