logoalt Hacker News

seba_dos1today at 10:32 AM6 repliesview on HN

> If the team squashes every PR into a single commit, this output reflects who merged, not who wrote.

Squash-merge workflows are stupid (you lose information without gaining anything in return as it was easily filterable at retrieval anyway) and only useful as a workaround for people not knowing how to use git, but git stores the author and committer names separately, so it doesn't matter who merged, but rather whether the squashed patchset consisted of commits with multiple authors (and even then you could store it with Co-authored-by trailers, but that's harder to use in such oneliners).


Replies

theshrike79today at 10:43 AM

Can you explain to me (an avid squash-merger) what extra information do you gain by having commits that say "argh, let's see if this works", "crap, the CI is failing again, small fix to see if it works", "pushing before leaving for vacation" in the main git history?

With a squash merge one PR is one commit, simple, clean and easy to roll back or cherry-pick to another branch.

show 8 replies
mcpherrinmtoday at 11:51 AM

Squash merge is the only reasonable way to use GitHub:

If you update a PR with review feedback, you shouldn’t change existing commits because GitHub’s tools for showing you what has changed since your last review assume you are pushing new commits.

But then you don’t want those multiple commits addressing PR feedback to merge as they’re noise.

So sure, there’s workflows with Git that doesn’t need squashing. But they’re incompatible with GitHub, which is at least where I keep my code today.

Is it perfect? No. But neither is git, and I live in the world I am given.

show 3 replies
arnorhstoday at 10:50 AM

The author is talking about the case where you have coherent commits, probably from multiple PRs/merges, that get merged into a main branch as a single commit.

Yeah, I can imagine it being annoying that sqashing in that case wipes the author attribution, when not everybody is doing PRs against the main branch.

However, calling all squash-merge workflows "stupid" without any nuance.. well that's "stupid" :)

show 2 replies
lamaserytoday at 11:55 AM

Squash-merge is entirely fine for small PRs. Cleaning up the commits in advance (probably to just squash them to one or two anyway) is extra work, and anything that discourages people from pushing often (to get the code off their local machine) needs to be well-justified. Just review the (smallish!) total outcome of all the commits and squash after review. A few well-placed messages on the commit, attached to relevant lines, are more helpful and less work than cleaning up the commit history of a smallish PR.

For really large PRs, I’m more inclined to agree with you, but those should probably have their own small-PR-and-squash-merge flow that naturally cleans up their git history, anyway.

I categorically disagree that squash-merge is “stupid” but agree there are many ways to skin this cat.

LinXitoWtoday at 1:23 PM

How does not squash merging deal with the fact that branches disappear when merging? What I mean is that the information "this commit happened in the context of this PR or this overarching goal" goes missing. When you squash, you use the one central unit of information management in Git: the commit.

filcuktoday at 10:44 AM

Having the tree easy to filter doesn't matter if it returns hundreds of commits you have to sift through for no reason.

show 1 reply