Comment by hinkley
8 days ago
This is why I use rebase before PRs, and despise squash. You are not going to remember why you wrote that code that way 2 years from now and all we'll have to understand bugs and identify Chesterton's Fence situations is the deltas and the commit history. If you squash them I have 400 lines of code you 'wrote' all at the same time and only have the feature request it was assigned to as context. Thanks for nothing.
The worst actor would write a new module and check nothing in until it kinda worked. I think it went along with the fragile ego that had people sneaking around fixing bugs in his code without talking to him about it first. He wrote convoluted code that exhibited Kernighan's law and he was about 10 years too senior to still be doing that shit. He bragged about how 'powerful' his code was as if that was a compliment instead of a harbinger. Many times I found bugs in code from the initial commit. Just... give me something man. Anything. Fuck.
Just because you tried random shit until you found the problem doesn't mean you have to fess up to it. You can tell any story you want that gets us from point A to point B now that you know point B is attainable. You can rearrange the commits the way you would have written it if you knew exactly what needed to be done. Drop 90% of the code you wrote and immediately deleted again, anything that doesn't support that narrative.
In law enforcement you have something called Parallel Construction. You can know a suspect is guilty by knowing facts that are not admissible in court. So you need to rediscover those same facts by the book. Grab his trash on trash day. Interview neighbors. Get enough circumstantial evidence to get a search warrant, then go find that evidence again.
We're the opposite. No rebasing your PR. It changes the hashes which we use for ci/cd. Keep it linear, show me the full history, show me where you merged main back into it, and then we force squash on merge.
Main then has a nice single commit with a reference to the branch it came from if you wish to see how it developed. Why would we want to litter main with your 100 commits? You own your code, and if you wish to dig into why you did something in a commit, go look in your branch history.
> Main then has a nice single commit with a reference to the branch it came from if you wish to see how it developed.
Yeah sorry that's a toy project. You have 50 people push commits for six years and see if those branches don't 'accidentally' get deleted. We need a different VCS tool to get what you're after there.
GitHub, Gitea, and Forgejo specifically store all commit hashes for PRs in the `refs/pull/` namespace. The end result is that 50 people pushing commits for six years and deleting all of their branches after squash-and-merge loses no information about how the code evolved.
The main branch remains compact with a linear history without the commit noise. Deleted branches can be restored, commits that otherwise do not exist in the history can be interacted with, and so on.
One of our repos at work (56 contributors) still has all the original branch commits visible on PRs that were squashed and merged in 2018, I've never noticed anything like that being deleted accidentally or otherwise.
You can just use --first-parent to skip past commits in merged branches. Squash merges complicate stacked PRs.
But I agree on not rebasing PRs. Another benefit to avoiding force merges is that it's easier for reviewers to compare changes since their last review.
Probably coulda used an example that isn't itself a fourth amendment violation that essentially requires perjury to accomplish. Also less euphemistically called evidence laundering. Not really a neutral example.
A more charitable case is that the source cannot be disclosed because it's an undercover agent or informant. What the parent describes is indeed evidence laundering.
I was in fact thinking of informants but was a bit fuzzy on my facts so I demured. Sorry it came out a bit of hashed.
So are you going to go out and commit a crime because a random person used a crime as an analogy?
I’ve done it over less.
2 replies →
Squash and merge (if that’s what you’re talking about) is only useful in a pr context. Each PR would actually be a commit if you we using the git-email model. Commits inside such PR are just snapshots.
Squash smaller commits?
If it's part of the rebase. If commit 3 in the chain doesn't compile and half the tests fail, that's not very useful for someone trying to figure out how an ancient bug nobody noticed for a year got into the code. If commit 4 fixes it, then 3+4 -> 3.
But if you add a function in commit 2, then realize three hours later than you just need to call another function, I don't need to see the function arrive, get used, half the tests written, the whole thing be deleted, and then a couple commits that do it the easy way and only needs 3 tests added.
That fakeout will be in the code tree forever and each new dev who is trying to grok the code will see that the line of whitespace on line 35 came along with a function that doesn't exist anymore and only really ever did on David's machine.
> This is why I use rebase before PRs, and despise squash. You are not going to remember why you wrote that code that way 2 years from now and all we'll have to understand bugs and identify Chesterton's Fence situations is the deltas and the commit history.
This forces people to work in a very linear fashion that doesn't match how people actually work.
A 400 line commit from a squashed PR should be very manageable when tracing a bug in the main branch, especially if the PR has a good description and review. Having a bunch of "fixed, added, deleted" commits all pushed into main seems like a disaster of noise unless you now force everyone to bundle perfectly reversable actions in every commit.
> Having a bunch of "fixed, added, deleted" commits all pushed into main seems like a disaster of noise unless
unless you skip non-merge commits when reading the history of main. And personally, I don't remember needing to read main's history more often than probably once a year, and even then mostly out of curiosity.
Also: having a bunch of "ticket resolved" commits all pushed into main seems like a disaster of noise, compared to simple "release 203", "release 204", etc. series of commits that comprise the main. Squash even further! Just as you don't need to track every small development change inside a feature request, you don't need to track every small feature or fix inside a full release. Right? You write a changelog (if you even write them) using those 400 merge-commits, then squash it into a one commit for you release, bang, clean history.
> And personally, I don't remember needing to read main's history more often than probably once a year, and even then mostly out of curiosity.
You're probably delegating that work to someone like me who actually figures out what the systemic problem is that caused the same class of bug to make it to production 5 times in the last 3 years. If you're a lead or a principal and still saying this ^ then you need to expand your skillset.
Bad luck doesn't happen very often. Mostly it's blindspots.
I will confess though that the sort of forensics I do is probably not divisable from the fact that I'm also the designated VCS surgeon on every project I've been on since 1998.
> "fixed, added, deleted"
I'm saying get rid of those before you invite people to look at the code. Keep the 'code review changes' one because that's comedy gold when the PR changes forced on you by some snowflake actually cause a production outage at 2:00 am.
That's a lot of tedium and commit theater and is what squashing PRs is for.
1 reply →
Also - at least in GitHub if you squash with the PR merge action in the UI - the original commit history is still available in the ref maintained by the closed PR yet doesn’t clutter your branch or tags namespace.
What are the numbers these days? What percent of projects are running on github versus say <choking noise> Atlassian?
1 reply →