Splitting up commits is totally underrated and seen rarely. I see colleagues over and over again plumbing 4 kinds of changes into the same commit. Good luck reverting the one change that caused an outage.
Not the GP but we tend to use decently small and meaningful commits to make PRs easier to review. They then get squashed, so you can only (easily) revert the whole PR.
Personally I’m happy with that. If the thing merged caused an issue, I’d want to revert the entire thing that was merged, not a subpart of it which would result in a state of the codebase that nobody reviewed.
If you’re very diligent about making good atomic commits that always pass all test that can work, but I find that squashing PRs (and PRs being relatively small) is a very good trade off
Sounds reasonable. Also, when squashing a PR would conflate unrelated commits then maybe those commits should have been separate PRs instead.
I've recently started to like squashing after usually preferring rebase merges, because with squashes I have an easy option to rewrite the commit message and edit out all of those meaningless "fix typo", "improve" and "now actually working" lines...
> I've recently started to like squashing after usually preferring rebase merges, because with squashes I have an easy option to rewrite the commit message
And why couldn't you rewrite the commit messages while rebasing?
EDIT: oh, you mean from the GitHub webUI, I presume. Got it.
> EDIT: oh, you mean from the GitHub webUI, I presume. Got it.
Yes, exactly, while reviewing and merging other peoples PRs. Technically I could still rewrite in other ways, but while squashing in the web UI was the most comfortable workflow.
You're not the only ones, but I can't understand this approach. Do people never then read the version history? It must be impossible to understand commits' diffs with the changes all squashed together.
Not the OP, but I think that the point of squashing every PR is that the reviewers/PR run the whole PR, not the individual commits. If you have a PR with 5 commits, 4 of which break the build and the last one fixes it, then merging that will be a problem if you need to git bisect later.
So the idea is really "what's the point of having a history full of broken state?".
> It must be impossible to understand commits' diffs with the changes all squashed together.
This would be a hint that your PR was too big and addressing more than one thing.
> If you have a PR with 5 commits, 4 of which break the build and the last one fixes it, then merging that will be a problem if you need to git bisect later.
And the answer is that you don't; each commit is individually testable and reviewable. Changes requested by reviewers are squashed into the commits and then merged into the project. Unfortunately, while the git command line has "range-diff" to ease review with this workflow, neither GitHub not Gitlab have an equivalent in their UI.
Well, I was obviously meaning that "workflows that squash the commits in a PR are workflows where each individual commit is not tested/reviewed separately".
Of course, if your workflow is different, then... well it is different. Doesn't make the "squash workflows" irrational.
> And the answer is that you don't; each commit is individually testable and reviewable.
How does this work in practice? Is every single atomic commit reviewed by someone? When do they review each of those commits? How many commits typically go into a PR?
> Changes requested by reviewers are squashed into the commits and then merged into the project.
So a reviewer finds the appropriate commit that their comment applies to, and then changes the actual commit itself? Who is the author of the commit at that point?
I'm trying to understand what you're talking about, because you seem to have something figured out, for a problem that every team I've worked on struggles with.
> Is every single atomic commit reviewed by someone? When do they review each of those commits? How many commits typically go into a PR?
1) yes 2) when a PR is submitted 3) it can be a lot for a huge project-wide refactoring, but generally I would say 1 to 5 is typical and up to 20 is not strange.
> So a reviewer finds the appropriate commit that their comment applies to, and then changes the actual commit itself?
No, the author applies the requested change and force-pushes once he has gotten all the requested changes applied.
> because you seem to have something figured out
Thanks! But it's not me—it's how Linux has used git from the beginning, for example. In fact it's the only workflow that is used by projects that still use email instead of GitHub/Gitlab PRs, but (trading some old pain with new pain) it is possible to use it even with the latter. The harder part is marching the review comments to the new patch, which is actually pretty easy to do with emails.
It's quite some work and there's some learning curve. But depending on the project it can be invaluable when debugging. It depends a lot on how much the code can be covered by tests, in particular.
> So the idea is really "what's the point of having a history full of broken state?".
I rebase commits so they don't break the build but the history remains clean and incremental. Selective fixups and so on isn't the same as squashing everything into a single commit.
> This would be a hint that your PR was too big and addressing more than one thing.
I don't think so. Sure, that can be true, but squashes can also simply lose vital history. Suppose you remove a file and then replace it with code copied and modified from another file. If you then squash that, all Git will say is you made a massive edit to the file.
> I rebase commits so they don't break the build but the history remains clean and incremental.
Sure, and that's fine. The idea of the squash workflow is that they don't expect that. It's just different, and that's the rationale behind it :-).
> all Git will say is you made a massive edit to the file.
Which IMO is exactly what happened in this case xD. But again... whatever floats your boat, I was just talking from the point of view of a squash workflow.
I can only conclude that people who think squashing a work item into a single commit is great have never had to do serious bug hunting, relying on commit history for context, nor have they ever moved forges and lost the context from "the context is in the PR anyway".
I think it's a bit of a limited conclusion. Maybe they really just make small PRs that make sense, and maybe they rewrite the commit message into something useful when squashing.
My employer has all PRs merged by a bot once they're approved. The bot takes the PR description and uses that as the commit message. The PR is the unit of change that gets reviewed, not the commit. This makes for a nice linear bisectable history of commits (one per PR) with descriptions, references to issues on our tracker, etc. And no need to worry about force pushing, rebasing, etc, unless you want to do so.
Of course it's got the same end result as doing an interactive rebase & combining all the in-progress commits into a single reviewable unit of change with a good commit message, but it's a bit more automatic.
I think I've done all those things except forges. I don't know what that one is. I still like squashing. I've wven become the git expert on my team. With squashed PR resolutions, I can more reliably use bisect. Many individual commits were never actually meaningful in the first place.
Crafting good commits is a skill that nobody seems to care about. In fact, nobody really seems to care about learning git at all despite it being one of the main tools in software development.
I would even go as far as "crafting good software is a skill that nobody seems to care about". It feels like people care a lot about "being productive" though, hence tools like all the Copilots.
Crafting good software and being productive? I guess it depends. I am much slower at producing code than most of my coworkers, but as a result they spend very little time maintaining/debugging it. But I spend most of my time debugging their crap (and they do, too).
I think there is a point to make that better code is often more profitable in the longer term. But the norm (at least where I work) is to swim in crappy code all day and complain about it while producing more of it.
The incentives are to get the code out the door then improve things later, if it's worth it. There's a lot of risk in product development, it iften doesn't make sense to craft the perfect software when there's a chance that it won't see the adoption that was hoped for.
There is a difference between "crafting the perfect software" and "crafting good software". It is way too common to not realize it and settle on "crafting crappy software" instead. The problem being that crappy software is often very hard to improve later, if it's worth it.
I have lived through multiple examples of this, the biggest one being a bad protocol (that was provably badly designed at the beginning) that got adoption, many tools built around it, and ten years later everybody hates it and keeps complaining but it's so hard to replace because of all the tooling built around it.
And on this topic, another underrated feature that everyone seems to hate is merge commits.
If you use rebase on your own branch, but merge commits on the stable branch (whatever name you use for it), you get both things:
* Small individual commits with the size and ordering that the original author intended on their branch.
* A "commit group" that describes that set of changes for the sake of future maintainers' sanity, and that can be atomically reverted.
More so in PR-based flows. You can have `git log --first-parent` display only the PR merge commits[1], and `git log --no-merges` display the linear history.
[1]: Assuming you actually put useful info on those commits. If you use GitHub, those merge commits can be set to default to PR title and description.
I also set up recently the policy to only use merge commits on stable branch, as otherwise the path filter^1 in the workflows would not detect correctly which files changed after merging a PR.
> Good luck reverting the one change that caused an outage.
Surely you'd revert to the last good release? I'm sure you could revert a single commit and re-release, but it seems slower (you have to find the right commit to revert, and make sure that the result works correctly) and riskier (since now you're effectively rolling forward to a combination of commits that's never been deployed before)
commit.verbose true
This adds the whole commit diff in the text editor where you’re writing your commit message, to help you remember what you were doing.
It's not the only sane way, it's just what IDE zealots say... even though you don't even need an IDE to do it the "IDE way". There are plugins to highlight and stage diffs in most text editors.
Regardless I also really like `git add -p`. I don't use it often but it really makes me focus on and consider each hunk. It was the defacto way to do it at an old company I worked at and no, it was not a small startup "hipster" company, we built enterprise ERP software.
Maybe for some people? git add -p does everything I need, directly from the terminal which is where I use git already. It's also super fast, the only limit being how quickly I can parse each hunk and type "y enter" or "n enter". Very occasionally I want to split a hunk up more than it allows, but it's easy to work around.
This UX is laughably bad. It presents you every hunk, one at a time, without context, and if you want to include some lines and not others, you can't unless all the sub-hunks are exactly one line or you let it launch a text editor and you dick around in the raw diff which is its own domain-specific language.
Why don't people play starcraft like this?
(1/1) Move this zergling [u,d,l,r,b,a,J,g,/,e?]?
The fact that this needs a write-up and discussion is all you need to know. Where is the big how-to for committing partial files in GitHub Desktop? Oh, yeah, nobody needs one because it's obvious and right in front of your face.
If you have no limbs and can't use a mouse to click on things then this UI is great though.
It’s really easy to learn: one of the option is ‘?’, try it out, it will give you more explanation.
The ‘s’ to split changes is one of my favorite feature.
It takes a few minutes to get used to it, then becomes automatic: add the whole file ‘a’, skip file ‘d’, skip chunk ‘n’, stage chunk ‘y’, split ‘s’, stop here ‘q’, rollback ctrl-c.
The “chunk per chunk” approach is actually really nice, you’re focusing on what exactly you want to get your commit to be about.
You learn it once, that will never change, you’re set for decades.
It makes you more productive than not splitting commits at all. It doesn't make you more productive than doing it in a GUI.
It's like you're saying "guys you should use this new hand drill I've discovered! It's so much more productive than digging holes with an awl!" while ignoring the fact that electric drills exist.
Is there something unusual about your example that means (e)dit doesn’t work if you want to treat those lines as separate hunks? Usually if you have multiple logical changes that are adjacent and get recognised as a single change by default, you can just open your editor and keep the lines you want to stage, much the same as you’d do by marking the lines you want to stage in most of the interactive GUIs built on top of Git.
You shouldn't have to edit a raw diff which is its own language just to split two adjacent lines. I'm not saying it should be fixed, I'm saying people should stop using the command line to do this entirely. (I find it quite funny that they ever did.)
Oh, I forgot that it probably couldn't be fixed even if they wanted to because if split suddenly started working it would break someone's spacebar heating script.
It seems a stretch to call having a + character at the start of an added line “its own language”. Staging one line of the two in your example probably takes less than five seconds using this method; in the editor, you just delete the line you don’t want.
If you personally prefer a GUI then that’s obviously your choice. Some people do and I doubt you’ll find many people claiming Git has the world’s best UI. It does work everywhere, though, and I doubt it’s any slower than using a GUI for a task like your example. Either way, you can have lines staged or not pretty much as fast as you can read them and decide whether they should be.
It’s perfectly doable in the command line. Utilities like git-gui make it fairly easy to stage and commit chunks or lines. Git plug-ins for vim or eMacs (the article mentions magit) are also quite efficient. You don’t need a bloated IDE for this.
I do hg split (which I think this is based on? Or at least it's very similar) all the time and it's fine. I guess an IDE makes it a little easier to select regions that don't line up with hunks?
i'm going to have to try vscode again, is this Sync you mention something other than the usual sync'ing of the user profile? what does vscode do differently with branches?
Git GUIs are always changing the interface to git. It gives you this weird lockin and islandification. Knowing how to actually use git can give you super powers.
I'd be careful with that feature though because it used to have an absolutely awful bug (and it might still tbh, I haven't had it confirmed that it's fully fixed yet).
If you staged selected ranges it could change the line endings of the whole file from CRLF to LF. It's very easy to miss since it doesn't show in the normal diff, but then you end up with merge conflicts galore later on.
I've used tig to do this for 15+ years. It's effortless. I know lazygit is more modern and has more features, but tig does this so well I have no need to change.
I kept running into an issue with tig where it couldn't break apart a hunk, or otherwise couldn't stage a part of one. I never figured it out by the time I gave lazygit a shot. Now that I've used lg in anger I only wish I had invested in it sooner.
+1 for lazygit, and I say this as someone who has used git exclusively on the cli for years and still continues to do so. It's just that lazygit makes staging hunks, squashing and rebasing such a joy.
the only reason I do some git stuff in vim and not _always_ in the shell, is because tpope is very thoughtful and fugitive.vim provides nice ways to deal with hunks or hunk partials (visually selecting a range within a hunk, for i.e.)
I didn't know about git add -p when I started using magit and thought it was an amazing magit-specific feature and went around cluelessly telling everybody about it. Later when I was experimenting with a different editor I realized my folly, and have come to find that I prefer git add -p, since I often want to provide multiple paths or a glob etc when its time to add to the index.
Something I found that I like about the approach is that sometimes while working on a change, some unrelated code will catch my eye that I want to fix but not as part of my next commit. Being free to make the edit, knowing that I'll see it again later when staging changes, is nice because I would often otherwise forget to go back and revisit the thing I saw. The git add -p sessions then wind up being an opportunity to decide what changes go into each commit, etc. I do burn myself occasionally by having an unrelated edit that is in a hunk that can't be split- I suspect there are tweaks to diff I could make to help with that but have yet to overcome my apathy wrt research.
> I do burn myself occasionally by having an unrelated edit that is in a hunk that can't be split- I suspect there are tweaks to diff I could make to help with that but have yet to overcome my apathy wrt research.
`git add -p` allows you to "edit [e]" the patch, meaning that you can really decide exactly what should be added. And that's a great opportunity to learn how to manipulate patches, which is an important skill IMO!
well im a dope, I never noticed the edit option. So if I edit the hunk and toggle the subset of changes I don't want, I'll get prompted to accept the diff of the diff? brb I need to go try this.
I came here to say this. `git gui` is nearly ubiquitous and has saved me for years from learning how to use `git add -p`. I don't use many graphical git tools but its options "stage hunk" and "stage line" are more intuitive and fine-grained than the hunk-splitting in `git add -p`
I'm a native git guy in general, but `git gui` has absolutely won my heart. I don't know what I'd do without it for staging partial commits. That and searching through stashes.
Totally, been doing it for a long time in IntelliJ and the other JetBrains IDEs, still stunning colleagues by simply check marking blocks or lines to commit the way it conveys hopefully the most meaning, so there is little if any dependence between the logical units across the commits, so as to be able to revert easily if necessary. And then the PR tells a story rather look like a frightening blob of shuffled ASCII chars.
Also check out `git add --patch`'s cousins over in `git checkout` and `git reset`. I probably use reset --patch more than add, as I'll usually have a file full of relevant changes with only 1 hunk i want to 'unstage', so I'll stage the whole file and unstage the irrelevant hunk.
I use `git checkout --patch` mostly when splitting a big commit into smaller commits, retroactively. I start an interactive rebase, chuck 'break' before the big commit, then I start pulling the changes 'forward' from the big commit into the working directory with `git checkout --patch big-commit-hash */path.ext`. Sometimes this works out easier than popping a commit off and re-committing everything seperately, particularly if I already have a commit with a detailed message.
I have used patch-adding almost exclusively for a few years. It really helps me stay aware of my workspace and avoid committing temporary tweaks. I can also keep semi-permanent personal tweaks uncommitted in my workspace and get constant reminders if I still have an inline debugging flag activated.
It's unlikely to remember this feature's existence and benefit from it without habitual usage.
102 comments
[ 3.9 ms ] story [ 187 ms ] threadIf you’re very diligent about making good atomic commits that always pass all test that can work, but I find that squashing PRs (and PRs being relatively small) is a very good trade off
I've recently started to like squashing after usually preferring rebase merges, because with squashes I have an easy option to rewrite the commit message and edit out all of those meaningless "fix typo", "improve" and "now actually working" lines...
And why couldn't you rewrite the commit messages while rebasing?
EDIT: oh, you mean from the GitHub webUI, I presume. Got it.
Yes, exactly, while reviewing and merging other peoples PRs. Technically I could still rewrite in other ways, but while squashing in the web UI was the most comfortable workflow.
So the idea is really "what's the point of having a history full of broken state?".
> It must be impossible to understand commits' diffs with the changes all squashed together.
This would be a hint that your PR was too big and addressing more than one thing.
And the answer is that you don't; each commit is individually testable and reviewable. Changes requested by reviewers are squashed into the commits and then merged into the project. Unfortunately, while the git command line has "range-diff" to ease review with this workflow, neither GitHub not Gitlab have an equivalent in their UI.
Of course, if your workflow is different, then... well it is different. Doesn't make the "squash workflows" irrational.
Disclaimer: I don't squash PRs.
How does this work in practice? Is every single atomic commit reviewed by someone? When do they review each of those commits? How many commits typically go into a PR?
> Changes requested by reviewers are squashed into the commits and then merged into the project.
So a reviewer finds the appropriate commit that their comment applies to, and then changes the actual commit itself? Who is the author of the commit at that point?
I'm trying to understand what you're talking about, because you seem to have something figured out, for a problem that every team I've worked on struggles with.
1) yes 2) when a PR is submitted 3) it can be a lot for a huge project-wide refactoring, but generally I would say 1 to 5 is typical and up to 20 is not strange.
> So a reviewer finds the appropriate commit that their comment applies to, and then changes the actual commit itself?
No, the author applies the requested change and force-pushes once he has gotten all the requested changes applied.
> because you seem to have something figured out
Thanks! But it's not me—it's how Linux has used git from the beginning, for example. In fact it's the only workflow that is used by projects that still use email instead of GitHub/Gitlab PRs, but (trading some old pain with new pain) it is possible to use it even with the latter. The harder part is marching the review comments to the new patch, which is actually pretty easy to do with emails.
It's quite some work and there's some learning curve. But depending on the project it can be invaluable when debugging. It depends a lot on how much the code can be covered by tests, in particular.
I rebase commits so they don't break the build but the history remains clean and incremental. Selective fixups and so on isn't the same as squashing everything into a single commit.
> This would be a hint that your PR was too big and addressing more than one thing.
I don't think so. Sure, that can be true, but squashes can also simply lose vital history. Suppose you remove a file and then replace it with code copied and modified from another file. If you then squash that, all Git will say is you made a massive edit to the file.
Sure, and that's fine. The idea of the squash workflow is that they don't expect that. It's just different, and that's the rationale behind it :-).
> all Git will say is you made a massive edit to the file.
Which IMO is exactly what happened in this case xD. But again... whatever floats your boat, I was just talking from the point of view of a squash workflow.
Of course it's got the same end result as doing an interactive rebase & combining all the in-progress commits into a single reviewable unit of change with a good commit message, but it's a bit more automatic.
I think there is a point to make that better code is often more profitable in the longer term. But the norm (at least where I work) is to swim in crappy code all day and complain about it while producing more of it.
I have lived through multiple examples of this, the biggest one being a bad protocol (that was provably badly designed at the beginning) that got adoption, many tools built around it, and ten years later everybody hates it and keeps complaining but it's so hard to replace because of all the tooling built around it.
usually you should err on the side of keeping the commits together, unless you’re close to release, patching a bug in release, etc.
If you use rebase on your own branch, but merge commits on the stable branch (whatever name you use for it), you get both things:
* Small individual commits with the size and ordering that the original author intended on their branch.
* A "commit group" that describes that set of changes for the sake of future maintainers' sanity, and that can be atomically reverted.
More so in PR-based flows. You can have `git log --first-parent` display only the PR merge commits[1], and `git log --no-merges` display the linear history.
[1]: Assuming you actually put useful info on those commits. If you use GitHub, those merge commits can be set to default to PR title and description.
[1] https://github.com/dorny/paths-filter
Surely you'd revert to the last good release? I'm sure you could revert a single commit and re-release, but it seems slower (you have to find the right commit to revert, and make sure that the result works correctly) and riskier (since now you're effectively rolling forward to a combination of commits that's never been deployed before)
commit.verbose true This adds the whole commit diff in the text editor where you’re writing your commit message, to help you remember what you were doing.
https://jvns.ca/blog/2024/02/16/popular-git-config-options/#...
Regardless I also really like `git add -p`. I don't use it often but it really makes me focus on and consider each hunk. It was the defacto way to do it at an old company I worked at and no, it was not a small startup "hipster" company, we built enterprise ERP software.
(2/5) Stage this hunk [y,n,q,a,d,K,j,J,g,/,e,?]?
Wow, so intuitive! LOL. You really showed him.
This UX is laughably bad. It presents you every hunk, one at a time, without context, and if you want to include some lines and not others, you can't unless all the sub-hunks are exactly one line or you let it launch a text editor and you dick around in the raw diff which is its own domain-specific language.
Why don't people play starcraft like this?
(1/1) Move this zergling [u,d,l,r,b,a,J,g,/,e?]?
The fact that this needs a write-up and discussion is all you need to know. Where is the big how-to for committing partial files in GitHub Desktop? Oh, yeah, nobody needs one because it's obvious and right in front of your face.
If you have no limbs and can't use a mouse to click on things then this UI is great though.
The ‘s’ to split changes is one of my favorite feature.
It takes a few minutes to get used to it, then becomes automatic: add the whole file ‘a’, skip file ‘d’, skip chunk ‘n’, stage chunk ‘y’, split ‘s’, stop here ‘q’, rollback ctrl-c.
The “chunk per chunk” approach is actually really nice, you’re focusing on what exactly you want to get your commit to be about.
You learn it once, that will never change, you’re set for decades.
EDIT:
You have extremely low standards for what constitutes a professional tool these days.
You also didn't address my point. split is brain damaged and doesn't work:
You try to split this hunk and it won't do it. This task is beyond trivial in any non-retarded GUI.-p makes you more productive because you don't know how to use an electron app or Java app
It's like you're saying "guys you should use this new hand drill I've discovered! It's so much more productive than digging holes with an awl!" while ignoring the fact that electric drills exist.
Oh, I forgot that it probably couldn't be fixed even if they wanted to because if split suddenly started working it would break someone's spacebar heating script.
If you personally prefer a GUI then that’s obviously your choice. Some people do and I doubt you’ll find many people claiming Git has the world’s best UI. It does work everywhere, though, and I doubt it’s any slower than using a GUI for a task like your example. Either way, you can have lines staged or not pretty much as fast as you can read them and decide whether they should be.
https://github.com/microsoft/vscode/issues/96104
If you staged selected ranges it could change the line endings of the whole file from CRLF to LF. It's very easy to miss since it doesn't show in the normal diff, but then you end up with merge conflicts galore later on.
Magit's USP is that it's a great interface for those who already use (or are willing to use) `emacs`. `lazygit` is a great interface in its own right.
I use git rebase and git add/reset -p left and right.
I will definitely checkout lazygit though, thank you and others in this HN submission for the mention, looks useful.
https://github.com/tpope/vim-fugitive/blob/master/doc/fugiti...
But that's the beauty of it: git makes it easy for anyone to use their favourite tool (be it CLI, TUI or GUI) :-).
Something I found that I like about the approach is that sometimes while working on a change, some unrelated code will catch my eye that I want to fix but not as part of my next commit. Being free to make the edit, knowing that I'll see it again later when staging changes, is nice because I would often otherwise forget to go back and revisit the thing I saw. The git add -p sessions then wind up being an opportunity to decide what changes go into each commit, etc. I do burn myself occasionally by having an unrelated edit that is in a hunk that can't be split- I suspect there are tweaks to diff I could make to help with that but have yet to overcome my apathy wrt research.
`git add -p` allows you to "edit [e]" the patch, meaning that you can really decide exactly what should be added. And that's a great opportunity to learn how to manipulate patches, which is an important skill IMO!
I use `git checkout --patch` mostly when splitting a big commit into smaller commits, retroactively. I start an interactive rebase, chuck 'break' before the big commit, then I start pulling the changes 'forward' from the big commit into the working directory with `git checkout --patch big-commit-hash */path.ext`. Sometimes this works out easier than popping a commit off and re-committing everything seperately, particularly if I already have a commit with a detailed message.
It's unlikely to remember this feature's existence and benefit from it without habitual usage.