[diek]

> I die a little bit each time I try to understand what changes were related to a line when tracking down a bug

A change/feature/bug is a branch, which is squashed into a commit on your main branch, right? So your main branch should be a linear history of changes, one change per commit.

How does that impact the ability to git blame?

[js2]

Because unless it's the most trivial of features, you'll break it up into smaller commits which each explain what they are doing and make reviewing the change easier.

As a simple example, I recently needed to update a json document that was a list of objects. I needed to add a new key/value to each object. The document had been hand edited over the years and had never been auto-formatted. My PR ended up being three commits:

1. Reformat the document with jq. Commit title explains it's a simple reformat of the document and that the next commit will add `.git-blame-ignore-revs` so that the history of the document isn't lost in `git blame` view.

2. Add `.git-blame-ignore-revs` with the commit ID of (1).

3. Finally, add the new key/value to each object.

The PR then explains that a new key/value has been added, mentions that the document was reformatted through `jq` as part of the work, a recommends that the reviewer step through the commits to ignore the mechanical change made by (1).

A followup PR added a pre-commit CI step to keep the document properly linted in the future.

[diek]

In general I agree with you, there are absolutely times where you want to retain commit history on a particular branch (although I try to keep the source tree from knowing about things like commit IDs).

I would argue that those are by far the minority of PRs that I see. As I mentioned in another comment, _most_ PRs that I see have a ton of intermediary commits that are only useful for that branch/PR/review process (fixing tests, whitespace, etc). Generally the advice I give teams is, "squash by default" and then figure out where the exceptions to that rule are. That's mainly because, in my opinion, the downsides of a noisy commit graph filled with "addressing review comments" (or whatever) commits are a much bigger/frequent issue than the benefits you talk about. It really depends on the team.

[js2]

> As I mentioned in another comment, _most_ PRs that I see have a ton of intermediary commits that are only useful for that branch/PR/review process (fixing tests, whitespace, etc).

Right, but that's only because developers don't amend and force push their commits to the PR branch as they receive feedback. Which is largely encouraged by GitHub being a terrible code review tool.

To me, git is part of the development process, it's not an extra layer of friction on top. So I compose my commits as I go. I find it helpful for recording what I'm thinking as I write the code. If I wait till the very end, I'll have forgotten some important bit of context I wanted to include. So during the day I may use the commits like save points. But before I push anything I'll often check out a new branch and create and incremental set of commits that have the change broken down into digestible pieces. And if I receive feedback, I'll usually amend those changes into the PR and force push it.

I'd like to add that I spend a lot of time cleaning up tech debt. And I deal with a ton of commits and PRs that don't explain themselves. So I'm really biased toward a clean development workflow because I hope to make the lives of those who come after me easier.

I was also trained on this workflow by being an early git contributor and it had extremely high standards for documenting its work. There's a commit from Jeff King that's a one line change with about six paragraphs of explanation.

There's no right answer here. I value the "meta" part of writing code. Not everyone does and that's okay.

[throw555chip]

When the word "force" is involved, it's time to take a step back and re-evaluate things.

[js2]

It's due to GitHub lacking change set support. With Gerrit, force pushing isn't required.

[Izkata]

> only useful for that branch/PR/review process (fixing tests, whitespace, etc).

I have had bugfix cases where, digging through the repo history, both of those examples accidentally introduced the bug (the first because the person who made the original change didn't completely understand a business rule so it changed both the code and the test, the second because of a typo in python that only affected a small subset of the data). Keeping the commit separate let me see very quickly what happened and what the intent actually was.

[mablopoule]

Because now instead of having a line changed within a granular level of changes, it's lost with the other changes from the same feature branch, which is a more macro level. So if a change in config is needed for the feature, the part when this config change actually need to be handled, or would impact the data-flow is harder to evaluate now that you mix it with template changes, style changes, new interactions needed for the users, etc...

EDIT: On top of that, there's usually a bit of 'related' work you need for a task, by example when you find an edge case related to your feature, and now you also needed to fix a bug, or you did a bit of refactoring on a related service, or needed to change the data on a badly formatted JSON file.

Unbeknownst to you, you added a bug when refactoring the related service, a bug that is spotted a few months after, only on a very specific edge case. If the cause is not obvious, you might want to reach for git bisect, but that won't be very useful now that everything I've talked about is squashed into a single commit.

[diek]

> EDIT: On top of that, there's usually a bit of 'related' work you need for a task, by example when you find an edge case related to your feature, and now you also needed to fix a bug, or you did a bit of refactoring on a related service, or needed to change the data on a badly formatted JSON file.

I agree that's related work, but I'd argue that work doesn't belong in that branch. If you find a bug in the process of implementing a feature, create a bugfix branch that is merged separately. If you need to refactor a service, that's also a separate branch/PR.

That's actually the most common pushback I get from people when I talk about squashing. They say "but then a bunch of unrelated changes will be lumped together in the same commit", to which I respond, "why are a bunch of unrelated changes in the same branch/PR?"

[mablopoule]

I agree with you in principle, but it's usually because of process and friction. In the place I'm working right now, that would result in days lost as I need to create a new Jira ticket, which obviously require a team meeting for grooming (because Agile!), and then going after colleagues so that the PR is accepted, which best case still need for CI/CD pipeline to finally deploy, and then merge it to the dev branch, and finally rebase the current feature branch... and all this multiple times.

[joshribakoff]

Because sometimes a PR touches more code than a single commit, and you lose the more granular context surrounding the more granular changes. You can always ask git to make the log more coarse, but once you “destroy” the granular history it is for all intents and purposes gone.