Do we have a policy on whether to squash when merging a PR to the racket repo? My knee-jerk inclination is not to squash, it loses information, but I can see the case for squashing as well. Opinions?
Thanks!
John
Kind of. In the Style guide.
I most of the times squash before sending the PR.
For example implement something for vectors in a commit, then the same thing for strings in another, then for bytes. While adding the tests for bytes I get a brilliant idea and I add a few more test and I copy them for vectors and strings. In some case I separate the tests and then squash the new test with the old commits, n some cases it's a mess. Perhaps now refactor to use the same code for all of them. Or a new small feature for all of them. And now I have like 5 or 6 commits (and a few fixups). So I squash all of them before sending the PR.
In some cases it is possible to isolate the parts and I may choose to make two commits instead of a big one. (For example a new feature may expose an error in a signature of a function in Chez Scheme. I may fix the signature (+ tests) in a commit and add the feature (+ tests) in the second one.)
And also, there are the fixes and additions during the PR. Most of the time, I'll just squash them before merging. It makes the story easier to read in the future.
What does 'squash' mean in this context?
-- hendrik
Section 8, " Retiquette: Branch and Commit ? Not seeing anything there about squashing or not squashing? I should clarify, this is in re: merging PRs submitted by someone else. (And I'm not thinking of a particular case, just wondering in general.) I suppose "use your judgment" is pretty reasonable here.
When submitting to a git repo, a "squash" refers to the act of collapsing a sequence of adjacent commits into one commit. There is support in the command-line tools and in helper tools such as github for squashing a number of commits when making a PR. In some projects, this is considered de rigueur; every PR should be exactly one commit. It sounds like we don't have hard-and-fast rules about this.
My understanding from the style guide and observing behaviours in the GitHub - racket/racket: The Racket repository repo is that meaningful, independent commits with useful commit messages are preferred. Merge commits should be avoided. (Other repos may have different policies.)
So, when merging someone else's PR, you have two remaining options (since merge commits are out): squash or rebase. In my experience, it's usually clear enough which to choose when skimming the list of commits: if they appear to be independent useful chunks of work with good messages, use rebase to preserve them, otherwise squash.
Sometimes people (out in the wider world, I don't see it much in the Racket community) commit very frequently with low signal messages. In cases like that, it's best to squash for sure, as those commits are really just WIP milestones for the author which are unlikely to make sense to others.
2 Likes
I think I’m guilty of this sometimes.
This looks like a helpful post:
S.
That blog post looks over-simplified and kind of almost-wrong. Specifically, in my opinion the author really should have used an example where work continued on the main branch before the merge/rebase. I would also say that merge/rebase is almost entirely independent of squashing, I don't see what the interaction is. But ... okay?
I must have misread. It is also a 2016 post and the GitHub ui has changed dramatically. Regardless: I.Must. Squash. 
Edit: current docs About pull request merges - GitHub Docs
I think PRs normally should be squashed, because they normally correspond to one coherent change. There are sometimes larger PRs where keeping the commits makes sense, but that's not the usual case.
I can see why you'd ask, because PR #4676 is a good example where squashing is better. The commit messages in that case make sense as part of a PR revision process, but not as a part of the Racket source history. I imagine the PR author expected squashing in in this case (maybe because I almost always squash when merging).
Not wanting to lose information makes sense, but this extra information gets in the way. In fact, I'm motivated to reply because I was trying to get my bearings on recent commits, and the non-squashed merge made that a little harder. 
2 Likes
Sorry for the mess! I did intend to squash it.
That's been my assumption/expectation in the (relatively few) PRs I've made.
In fact my impression of the de facto policy is, "JIT squash".
-
While the PR has any ongoing conversation ("please change X") then I just push more commits with the requested changes. I figure that detailed history might be easier to follow, during review? So I wouldn't want to squash too soon.
-
When things settle down, I might force-push a squash. But normally I'd leave that up to the merger. I figure it's their repo, and really up to them how they prefer the history for their main branch?
(Personally, I like the squashed, "linear" history style the racket repos use; I'm not a fan of merge commits, for most projects' main branches. But if some repo did prefer that, that's their call not mine.)
3 Likes
Okay, I think I hear a consensus preference for squashing, many thanks.
Side note: I can't tell whether this is genuine curiosity or the desire to find errors in others' statements, apologies if it's the latter, but @greghendershott you also appear to be suggesting that the two choices are (squash-and-rebase) and (dont-squash-and-merge). Is there some reason that (dont-squash-and-rebase) isn't an option? Let me reiterate that I do think that I see a clear preference for squash-and-rebase here, I just feel like I'm missing something.
Oh! I think I'm starting to see the issue here, it's a GitHub interface choice?
IOW, GitHub doesn't give you squash-and-rebase. That's weird to me. In fact, they say "Rebase and Merge", which is more or less a direct contradiction, using those terms in what I'm going to snootily call the classic git sense.
"Squash and merge" attempts a fast-forward merge -- which will fail if there is a merge conflict.
IIRC, if the branch needs to be rebased (to enable doing a fast-forward merge), the GitHub UI will have already warned about that, and offered a button to click to attempt the rebase.
Oog. Sounds to me like GitHub is just making up terms here. When they say "fast-forward commit", I think they're saying "a rebase with no conflicts". That's just not a merge. Specifically, a merge commit is one with two antecedents. I see no reason for them to muddy the waters by using the term "merge" for something else. Bleah.
Derp. Not their fault, mine. I meant to write "fast-forward merge".
And "fast-forward merge" is git's term.
TL;DR: Although GitHub may deserve blame for many things, this isn't one.
Squashing commits already grows a new commit containing all the changes in the PR, so there is no need to modify the original commits in the PR. Rebase and merge re-grows all existing commits in the PR to concatenate them on top of the main branch. The typical merge-commit-strategy is disabled as indicated in the top-most option.
Right, but you could certainly squash and then have a non-trivial merge commit, and I can even imagine this being a sensible choice for some workflows.
I can see (in my own head) that this is gradually shading over from "what's the right thing to do" to "I need to prove that I'm right", so ... I'm happy to let this go.
Oh, wait, I know what I'm supposed to say here:
"Hmpf"