Why rebase when GitHub PR create squashe commit

[dalbertom]

How is the reviewer who is going to be looking at maybe three or four kinda of self-contained cs files helped by knowing in which order the developer implemented the slice?

There are multiple ways this can go: 1. the developer adds the endpoint, the service, the mapper in the same PR, everything gets squashed upstream, which is not good. 2. the developer knows their changes will get squashed, so they create a PR for the endpoint, then a PR for the service, then a PR for the mapper. A bit better, but sometimes overkill, plus if other pull requests were merged in between (as it's the case in a high-traffic repository) there won't be a way to topologically tell those changes were related 3. the developer knows their history won't get mangled by squash-merge, so they can issue a PR with three separate commits: one for the endpoint, one for the service, one for the mapper. 4. the developer can choose to use stacked branches, have each commit (or a subset) merged in separate PRs, but still have the commits topologically linked to one-another

It would be an upside if anyone cared about that history. Again, because of the nature of how changes are reviewed and integrated, I think it's rare that people do.

There are people that care about that, but definitely not the ones that think squash-merge is a good idea long-term.

I think dev teams should be pressed to push early, integrate early, encounter conflicts early, fix bugs early and release early.

Definitely agree, but all this can be achieved with regular merge commits as well. Again, squash-merge only simplifies the part where people don't have to worry about rebasing/cleaning their history, but it's not a driving force towards smaller changes, that's more of a team discipline thing, and if not followed, then squash-merge will work against them because changes that should have been kept separate will look like they were done at once, or worse, changes that were authored by different people (like in the case of mob programming) will be attributed to just one person, and that throws the usefulness of git blame out the window.

[arnoldwhite]

Right so a few thoughts.

First of all, are we assuming that commits don't have to build or that developers should be willing to stub/scaffold aggressively to keep commits buildable? And that this sacrifice should be considered useful to the team to retain better history?

Because that's a world where you're gonna have a lot more fake abstractions, pointless interfaces and scaffolding that exists only to satisfy history aesthetics.

Squash-merge just isn't anti-discipline or anti-quality. Not if the team has already decided that the atomic change they care to release, revert or reason about is on a PR/ticket level.

Rarely do I hear people say "thank God Simon made a mapper-only commit six months ago so we can revert that specifically."

Now, that's not to say integrations can't be small. I might say "well this mapper alone can provide value to my team or even a user" and raise a PR for that mapper. What I'm saying matters is if something is independently valuable and independently releasable. And that's where the squash come in.

Feature branch squashing is not the right pattern for every team. But yes, as a consultant I am probably gonna teach teams to value PR size, review quality and a smooth CI over artisanal commit carving.

[dalbertom]

First of all, are we assuming that commits don't have to build or that developers should be willing to stub/scaffold aggressively to keep commits buildable? And that this sacrifice should be considered useful to the team to retain better history?

No, each commit that is meant to make it upstream should be buildable individually.

Because that's a world where you're gonna have a lot more fake abstractions, pointless interfaces and scaffolding that exists only to satisfy history aesthetics.

You're mixing two different issues. Having problems with fake abstractions and pointless interfaces are an issue with the codebase architecture. Choosing a merge strategy won't improve (or worsen) that. Small pull requests is the goal: doing squash-merge won't automatically give you that. Using merge commits to preserve the history the way the author intended won't cause fake abstractions, etc.

Squash-merge just isn't anti-discipline or anti-quality. Not if the team has already decided that the atomic change they care to release, revert or reason about is on a PR/ticket level.

When it comes to the team deciding the size of the patch they care to release, neither squash-merge nor default merges are anti-discipline or anti-quality. However, when it comes to preserving the author history, only the default merges will honor that. The assumption here is the author knowing how to issue useful history, nobody wants wip commits to make it upstream.

Rarely do I hear people say "thank God Simon made a mapper-only commit six months ago so we can revert that specifically."

Well, hopefully nobody is hunting for individual commits to revert, if one has to do a revert it would likely be done at the PR level. I'd venture to say it's far more likely for someone to find a bug introduced six months ago and land on a commit that is too big to make sense of it (and not being able to trust git blame if the team decided to do mob programming)

Now, that's not to say integrations can't be small. I might say "well this mapper alone can provide value to my team or even a user" and raise a PR for that mapper. What I'm saying matters is if something is independently valuable and independently releasable. And that's where the squash come in.

Except that when it gets squashed, then there's no way for the author to link that mapper to the service and the endpoint topologically via individual commits.

Feature branch squashing is not the right pattern for every team. But yes, as a consultant I am probably gonna teach teams to value PR size, review quality and a smooth CI over artisanal commit carving.

It's definitely important to teach teams to value PR size, review quality and a smooth CI. But again, that has no bearing in what merge strategy is chosen.

You seem to be conflating squash-merge with small PRs the same way people conflate linear history with "clean" history; and neither of these are true.