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

81–90 of 350 posts

Re: Squash your commits

#81

Post by phasmantistes »

jldugger wrote: post #72
phasmantistes wrote: post #69

Earlier quoted context omitted.

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

> It means that the code review process is more important that the code writing process. If that were true, the optimal solution is PRs with individual commits that all pass testing. I find it much easier to review a series of small changes for logical correctness than mashing them together into a single PR. Github recently added this as a feature, so I'm not in a completely invisible minority there. And then, when t…

Absolutely. If someone formulates their PR such that every commit in the chain is small, easily reviewable, and passes all tests, that's fantastic! That makes reviewing code, searching history, and bisecting all easier.

Unfortunately, that's not the 90% case that I see. Most of the time a multi-commit PR contains N-1 commits of incremental development and one final one that fixes all the tests and typos and removes debugging print statements. Neither the project nor the author benefit from having those intermediate commits integrated verbatim.

Re: Squash your commits

#82

Post by dasil003 »

scrollaway wrote: post #56
natrius wrote: post #53

Earlier quoted context omitted.

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.

Exactly. In order for always-squash to work, then every PR must be the smallest possible atomic change. But often times certain features don't degrade to nice small atomic changes. It's sometimes useful to see that as a development process smell and consider using feature flags and other things to incrementalize the development, but sometimes the cleanest thing to do is just have a series of commits. I don't see a good reason to take this option off the table since source code history management is truly a craft in its own right.

FWIW, I prefer a carefully curated rebase and then merge --no-ff so that you can still see the branch and get at the branch diff trivially, but the history is still linear for practical purposes so bisecting is clean, etc.

Re: Squash your commits

#83

Post by WorldMaker »

dap wrote: post #74
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 understand the point of view that it's useful to see what "actually" happened. But the other way to think about it is like this: at some point, when a developer in the future wants to understand how #master evolved over time, should the burden of linearizing the history rest with that developer (making sense of a complex graph), or with people who make changes to #master when they make them?

Obviously, -no-ff merge commits give you big sign boards for groups of changes today.

Other than that, maybe all of this is an indication of a need for a meta-UX over the change graph to annotate and describe subgraphs in new ways.

Re: Squash your commits

#84

Post by codingWithGit »

Pxtl wrote: post #46

This is a presentation issue masquerading as a data issue. If somebody suggested deleting data because a report was ugly, they'd be laughed out of the room. Give us tools to mark commits as unimportant or group them together as a meta-commit object for history purposes.

This is good insight. makes a lot more sense. I think this will take care of 99% of use cases for sqqshing/rebasing.

However, I think this is something that will have to be built into git itself, not by Github.

Re: Squash your commits

#85

Post by jh3 »

munificent wrote: post #30
MBlume wrote: post #26

If everyone on your team actually knows how to use git, much better to let them rebase their commits and mark out a series of clean, atomic commits which introduce the feature you're reviewing. If you have people who are incompetent at using git on your team, this feature will help protect your history from them.

> incompetent at using git This unfairly places the blame for Git's utterly shitty UX on the part of the users. When you have thousands of users who struggle to use a tool correctly, it's the tool's fault, not theirs. I've been using Git for years, work professionally full time on an open source project that lives on GitHub, maintain several open source projects with a number of committers and generally live and brea…

> I still fucking hate rebasing and get tripped up by it on the few times I end up having to deal with it.

When I had no understanding of what was going on, I didn't like it either. Now that I use it frequently, I understand it better, so I don't hate it anymore.

I like running `git rebase `, where is typically master, in my-new-thing branch because it lets me deal with any conflicts from upstream one by one.

I also like running `git rebase -i` in my-new-thing when I have a bunch of commits with redundant messages that I want squashed into a single commit before I push the changes. Basically anything that requires messing around with a range of commits is a good use case for `git rebase -i`.

Why do you hate it so much? There's really not much going on that you should have to hate. To me it's like a bunch of small, compartmentalized merges.

Re: Squash your commits

#86

Post by pkamb »

glhaynes wrote: post #33
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.

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.

That's the `diff` tab on the PR

Re: Squash your commits

#87

Post by codingWithGit »

gus_massa wrote: post #12

Perhaps I'm greedy, but I'd like also an additional option to rebase without squashing ... Also, it's not clear if it is possible to disable the merge button completely. I prefer to use the command line to rebase and fix the details in the commits, but the big green "merge" button is always too tempting and it's easy to press it by mistake.

you can always do this in the command line locally.

Re: Squash your commits

#88

Post by geuis »

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

Couldn't disagree more. Having worked extensively on teams on both sides of this issue, I can experientially state that a well-done git rebase and commit strategy is much more useful and helpful.

In terms of feature branches:

The individual engineer is free to do individual commits in their branch as they need to in order to keep track of their work. Before they submit a pull request, they should rebase and squash all of their commits into a single one that thoroughly describes everything in the feature that is being committed. When used in conjunction with tools like Phabricator, Arcanist, and commit templates, the workflow is very smooth.

When another team member goes to code review their pull request, rather than having to examine multiple individual commits there is only a single one to examine and comment on.

Master history:

Rather than cluttering up the mainline history with 'Did this', 'Did that', 'Merged: Did this', 'Merged: Did that', 'Reverted: Merged: Did this' etc, you get a series of commits that articulately describe what each commit was for. In the event you need to revert a feature because it breaks something, its much easier to revert that single commit than trying to hunt through all of the individual commits from an engineers feature branch. And in that case, if you revert one of the commits from the feature branch it could break something else.

Re: Squash your commits

#89

Post by WorldMaker »

ealexhudson wrote: post #71
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.…

Sometimes the scaffolding should be left in place to help show others how to build things; other times, it was put up and taken down so many times that the learning from the first few attempts isn't worth it. I would probably be much happier not squashing stuff if you were able to bisect cleanly to points between branch merges. I don't think that's even theoretically possible; which means that when you're bisecting i…

Maybe bisect needs a flag like --start-with-merges to focus it's efforts on -no-ff merges?

Also, what if there was a tool on top of bisect that could better utilize GitHub PR JSON to target the search pattern? That could even save you some time in the case where you already have CI information attached to your PRs...

Re: Squash your commits

#90

Post by stingraycharles »

jldugger wrote: post #72
phasmantistes wrote: post #69

Earlier quoted context omitted.

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

> It means that the code review process is more important that the code writing process. If that were true, the optimal solution is PRs with individual commits that all pass testing. I find it much easier to review a series of small changes for logical correctness than mashing them together into a single PR. Github recently added this as a feature, so I'm not in a completely invisible minority there. And then, when t…

I agree with what you're saying, good code review and good code writing aren't mutually exclusive. However, the truth of the matter is that code review is a far more clearly defined moment in a workflow than writing good code (which is more of a habit than an actual action). As such, it is far more efficient for a team to accept reality and squash after PRs (which is now first possible), instead of relying and keep correcting all your coworkers when they don't properly squash commits before pushing.
Post reply on HN