My two cents: I just always squash commits when merging to master/main. That way the main branch has clean commit messages and it's easy to revert later. I think cherry-picking commits after the fact can also be very time-consuming, especially if you do a lot of refactoring or "clean as you go" as I like to call it. However, I could see it being worth it for open-source.
When you go grocery shopping, you can A) create a list of dinner ideas and the ingredients that you need to pick up, group them in the order that they appear in the store, and head straight to the aisles to get the items. Or B) go in with the list in mind, but wander through the whole store and you may find something else interesting to pick up. Or C) not have any but the most vague dinner ideas, go without a list, and create a week's worth of dinners as you browse picking up interesting items and figuring out what to do with them.
All depends on if you want the most efficient approach, or if you want serendipity to strike.
I'm not sure this gives the right impression. Yes rename/squash/interactive rebase if necessary to tidy, however I still believe you should strive to create clear, separate commits as you go.
If you have to regularly make major changes to history before review, I could be a sign that your process/approach is disorganised.
> If you have to regularly make major changes to history before review, I could be a sign that your process/approach is disorganised.
Of course my process is disorganized! I'm an extremely disorganized person. I like to try out wild tangents in my coding, experimenting with some crazy idea or another and saving my place before and after I do so. I even have a separate .txt file of stream-of-consciousness writing about my coding, to keep track of the tremendous amount of complexity I have to deal with on a regular basis.
My reflog reads like a personal diary of failings, dead-ends, "FINALLY COMPILES!" commits, etc etc. I use `git branch someNewBranch` really often, keeping a namespace of local branches under the name `deadend/*` to mark tombstones of approaches I tried in code but didn't work out, and I `reset --hard` back to a previous commit after I save the branch. I like it this way, it maps better to my totally disorganized brain.
But I'm also a firm believer that none of my colleagues should be able to tell how disorganized all of this is, because by the time I make a PR, I squash it all down and write a very long, detailed commit message of what exactly I did, why I'm doing it, and how it works (sometimes, albeit rarely, spread across multiple logical commits in one PR.) They never get to see my private commit history.
Why does it matter to anyone how much I have to prune my commit history before a PR? That's like complaining to another student in class that their short-form handwriting is hard to read, in their own personal notes they're taking during class, when said student is acing all the tests. It's simply not the metric you should be judging people on.
Why commit at all if you're going to reset? Sounds like the intermediate commits are just used as a backup method. The real trickery is mentioned in the last part, if commits don't break up tidily along file boundaries you'll have to commit hunks somehow and then go back and test whether the code works.
Either way, in the end it's up to developer discipline to make clean commits, git doesn't really help all that much although in my (long) source control history before git I never thought of committing hunks.
Because committing is useful? It saves my work. I can get to a point where my code finally compiles, make a quick commit, then do some more dangerous refactoring. If something breaks, I can `git diff` to see what changes I've made since it last compiled.
I'll turn this around and ask you: Why do you feel like you can only make a commit if you have something that's reviewable? Is your code writing style so perfect that it all works the first time? Or do you just not bother committing anything until your work is done? (That personally sounds terrifying to me... I really need to be able to say "what have I changed since 10 minutes ago when this compiled?" on a very regular basis.)
Oh, I totally agree with all your use cases for committing, I'm just wondering why the reset. I would either go off on another branch to preserve my temporary work and then clean that up and commit or merge to one of the main branches or I would just push all my intermediate work.
The reset is just a straightforward way to squash your work into one commit without much fuss.
A typical workflow for me:
$ git commit -a -m "WIP" # picture this, but 100 times as I go
$ git reset $(git merge-base main HEAD) # Reset to the merge-base to avoid reverting upstream work
$ git diff # Get a nice overview of everything I've changed, to aid in crafting a nice commit message
$ git add .
$ git commit # Type up a nice commit message describing the whole change
Contrast this with using `rebase -i` to squash:
$ git commit -a -m "WIP" # picture this, but 100 times as I go
$ git rebase -i $(git merge-base origin/main HEAD) # rebase to the merge-base because I don't want to deal with merge conflicts yet
$ # search/replace 'commit' with 'squash' in a text editor
$ # save/quit
$ # get prompted for new commit message, type it out
$ # save/quit
They both feel like the same amount of typing to me, but `rebase -i` thrashes my working tree around with each commit, which has a habit of confusing my IDE quite a bit and breaking its build cache. Also, I like the `git diff` step in the reset workflow because it gives me a nice reminder of what my change looks like, which helps inform how I should word my commit. So I use reset.
If I want to keep my intermediate work locally for posterity, I usually use `git branch prprep/<bug_number>_unsquashed` or something so that I can dig it up later if need be. I don't push any of my `prprep/` branches.
Yeah I also don't see the need for it. Maybe if you often commit completely broken code with meaningless messages... But usually most commits we do at work are still proper little changes (even if it's just 2 lines of a huge feature) with meaningful information in the git message. I would not want to lose that when merging back into the main branch.
> Maybe if you often commit completely broken code with meaningless messages
Bingo. I do this all. the. time. I don't want anybody seeing it. I honestly couldn't imagine another way of working... commit early, commit often, I say. Hell, commit on a timer every minute, if that's your thing (I've never done this but totally understand people who do.) Leave the meaningful messages for later when you're actually ready to craft a PR.
Honest question: if you avoid committing code unless it's working and you have a meaningful message to make, what happens if you screw up your editor undo, and you need to go back to where you were 10 minutes ago? This happens all the time for me... I undo a whole bunch and then accidentally type the "z" button into my buffer because I fumbled the keyboard, and now I can't "redo" back again. If I commit early and often, I can recover the work to where it was, even if it's not compiling yet. If I have to wait for my code to be basically "reviewable", I'd be screwed.
Of course I'm talking about local commits, who brought up pushing? I only push when I'm ready to. But turning my local WIPs into something I can push, is most easily done with the reset workflow (but to each their own.)
It occurs to me that people like me might actually be quite rare, who use git almost entirely locally and rarely push. The only times I push are when creating a PR (hence all the resetting and rewriting first), or to back up my WIP work, if it's important. And even then it's to my own fork, and under a branch namespace that nobody would ever confuse with real code that should be reviewed.
I think this makes a lot of sense. I usually use git in a completely different way when developing than I do when I'm pushing up code for others into a shared branch.
When developing my priority is to easily get back to a last known working state. This allows me to try out risky changes, and throw them all away if it doesn't work out.
When pushing my changes for PR, those working states I saved before may not be the best logically. I usually re-write my commits to break it into more logical chunks, that are easy to revert and easier for other teammates to digest.
What a waste of time and effort. Squash when merging to master and be done with it. Every PR/commit merged to master should be a clean logical unit. That means not fixing a bug in a branch where I'm doing something else. Those will be two separate PR's. The second problem shouldn't exist IMO.
Bingo! This strategy has worked well for my org (75+ engineers) for years. If an engineer makes non-related bug fixes in the same branch we require them to revert the change and make a new branch. We also have each GitHub repo configured so the "Squash and Merge" option is the ONLY option available when merging a PR.
I don't care one lick what someone's branch history looks like. If they want to commit every day, or every hour, or after every keystroke - I don't care. All I know is that once the PR is merged, it's all going to be squashed into a logical unit so the `main` commit history will look just fine.
Dont make work upfront, that is rarely important. When it is important, find the commit and split it up (as necessary) at that singular point (instead of across all PRs). You have now cut down how much time it takes to make PRs while retaining the same end result.
The problem comes when you have a related set of changes where you both want to see how everything eventually fits together and where you still want to keep small "clean logical units." You can see this kind of thing play out frequently in, for example, Linux changesets, where you might have a 24-patch series of changes that need to go in for a feature.
There's no such thing as "clean logical units". There's a product you work on. There are bugs. The prodct needs some features, good UX, performance requirements.
Spending effort on managing git is mental effort you don't spend on solving your actual problems. By far the best experience I've ever hadeith git was: everyone works straight on the dev branch, just rebase, fix your stuff, test often, and if you're doing some multi-day work then sure, branch and think it over then merge.
That's it. That's all you need. I've had a million more problems with every attempt at making this process "clean", or "smart". Dumb was by far more efficient, more enjoyable, helped us find and fix bugs faster, and had the shortest time to market ever.
If you're in a large organization, you're expecting other engineers to make sense of the work you're submitting. Anything that helps them here is a good thing (although it's always a tradeoff.)
The advice I try to live by, is that however messy my work was leading up to a PR, I make sure the end result is something somebody can review without additional context. Commits should have lengthy descriptions of changes that describe the "why" and "how" of a particular change, in a way that makes it easy to digest for a reviewer. Sometimes multiple commits make sense (like if you're renaming a module/class, put that in a single commit, then put the actual code change in the next one), sometimes they're not necessary. But it's worth it to put in the effort here if it means it helps a reviewer, IMO.
I think you underestimate just how much stupid garbage I have in my commit history. It's embarrassing. Even if you squash when merging the PR, the squashed commit message is gonna have a ton of messages that look like "Why the hell isn't this compiling?", "wtf?", "FINALLY PASSES!" etc etc etc.
I could avoid creating those commits in the first place, but asking me to only commit my local changes when I have something intelligent to say about them, is a damn near impossibility for me. I basically use git to "save my work" before I try an approach to something. I reset back if it doesn't work out. Sometimes I reset back again, if the approach that didn't work out turned out to be the least worst option. I create commits to experiment with something, so that I can quickly compare back and forth between two approaches. etc. etc. etc.
I would instead say, that if you're only making commits when you have something logical to say, you're probably not using git to its fullest potential. You should really go nuts with it IMO, and only bother to sound intelligent in your commit messages once you're ready to do so, and (most importantly) you should still make commits before that happens. Git's decentralized for a reason, take advantage!
It's true that messy commits happen, that's why squash is there.
Nothing forces you to keep the messy commit messages either - keep the short commit message and make sure it's good, then delete the combined individual commit messages from the long message field, done.
My point is that I don't want anyone to see my WIP commits in the first place. Squashing only in the end when merging to master means the code reviewers get to see my messy commit history, which is what I'm trying to avoid. (Yes, my commit history is that bad that it's embarrassing. But at least I commit early and often, which has saved my ass more times than I can count.)
Not in gitlab at least. Click on squash and you see nothing but the total of the branch difference, the merge req title, and a few lines of boilerplate it adds.
About as easy as it gets. As titles are issue numbers changes are documented automatically.
I don’t want anybody to be able to see my WIP commits, even if it’s easy for them not to see them.
Yes, that probably is a “me” problem, and I’m too anxious about it for no reason, but that’s me. But if you saw my local scratch branches you’d probably understand.
(My wip commits occasionally bitch about coworkers, for example. Or they just contain a bit too much profanity for a professional environment. Or there’s just 50 worthless “WIP” commits with no other description. You’d never know any of this from my PRs.)
Recording sensitive information then relying on your perfect performance to remove it is flawed opsec. Not to mention a waste of time, as our great ancestor mentions.
It’s not a career ending thing if somebody saw my commit history. It’s just mildly embarrassing. It’s not a waste of time either… time spent crafting a final commit from the whole change, writing a descriptive commit message, and documenting everything well helps people review it, and that’s time well spent.
Ok, that's one strategy. However, we put that information in our issue tracker, docs, and source code comments, where more people can reach them more easily. Commit msg is PROJ-1234, which gets automatically gets linked to issue with design and comment history and project/product folks can contribute.
Sure, that’s exactly what I’m advocating for? I’m confused.
You said it’s a waste of time to do this and to just squash when merging instead. I’m saying I’d rather squash first so that my PR looks clean and doesn’t contain my WIP commits. Then you respond saying “well you can just squash before your PR then”… are we going in circles?
Your use case is a bit different than what the article is arguing for. If you have a personal preference to have everything shown as a single commit in the PR by all means do things the way outlined above.
My original comment was targeting the two supposed problems that the article is framing and the supposed convoluted (and possibly dangerous) solution with git reset.
> If someone else is doing the merge and you’re unsure if they will you can preemptively squash your whole branch so there’s only one commit in the pr.
I agree! And that's what I do. But OP said that would be "a waste of time and effort", hence my reply.
If you find a new bug, git bisect can very quickly find which commit introduced the bug, and understanding the origin makes fixing the bug easier. That mechanism does not work nearly as well if some commits are not in a usable state.
This does not work well if you have long-lived branches (i.e. weeks) for more substantial features. Completely squashing it would lose all the granular commits and especially their commmit messages, which might be useful for debugging later on.
I don't quite follow. E.g. at work with have feature branches with dozens of meaningful commits that we worked like 2-3 weeks on before merging to master.
My point was that I don't want to squash the information contained in all those commits.
I assume people here have different workflows, where they work alone on small features for maybe 1-2 days and then just squash all their tiny WIP commits into one?
You are implying the original branch holding that history will be deleted when squashed into a single commit for a PR. That's not the case (granted, that's based on settings on GitHub, Azure DevOps whatever you're using, but the option is there) because in the PR you still see which branch the changes come from. If you're not deleting the branches you're good. Even if you delete the branch when merging I think most PR views nowadays will keep that info (the commits) in perpetuity. Again, I don't think this is a problem.
Are you sure? My common usecase when debugging something is "Why is this line of code like this, and who might know something about it?". So then I git blame that file and see when exactly that line was modified. If that line is inside some huge blob of changes (from weeks of work) inside one commit, that is less useful to me. Even if the original branch is still floating around somewhere, that would still be an extra hassle.
> If that line is inside some huge blob of changes (from weeks of work) inside one commit, that is less useful to me.
Yes, that's true. My original point still stands though. That commit will have a PR associated with it. You can go to it and see the details of all commits that were used for the merge commit that went into master. Not really a lot of extra work to go to the PR if you're investigating that deep.
> Even if the original branch is still floating around somewhere, that would still be an extra hassle.
If the branch deletion setting when merging a PR is selected, then only the origin branch will be deleted. If you're the one doing the PR and you don't delete your local branches you'll be able to see it locally. In the other case where is someone else's then it becomes harder, and I agree it's an extra hassle.
In my work I try to mandate PR's to be as small as possible, but not smaller. Big PR's do happen, but that and resorting to git blame or bisect happens maybe 2-3 times a year. I think the general rules for source control followed by the team are more important than the implementation details and git magic.
> Every PR/commit merged to master should be a clean logical unit.
The issue is one of review scaling. I wrote a blog post about this a while ago[0], but the gist of it is that those clean logical units are often too small for meaningful high-level reviews of more complex work.
With complex features or refactorings, you're often in a situation where those clean logical units allow reviewers to do a good low-level review (do a check for logic corner cases, style issues, etc.) but they don´t allow a high-level review of how all the pieces of the feature work together.
IMHO the most open-source process friendly solution to the issue is to review patch series, where you can review the series as a whole for the big picture, but also dig into individual commits for the details. Building such a patch series requires an approach as described in the article.
(In closed source environments, you may get a good enough approximation of the result with a separate, disciplined software design process.)
This to me seems more like a logical separation than anything technical.
I use GitHub and always squash commits before merging a PR. This keeps the commit log clean and also has the side effect that you have the PR number in the merge commit.
Having said that I also suggest keeping PRs small. If you are going to reformat a code base, make that a separate commit. Updating a library, separate commit. Adding a library you need for a new feature: create a PR for the library update, base your implementation off that branch and rebase against main when the first PR is merged.
+1 to keeping PRs small. This makes it much more logical to me. But I have worked at organizations where this is frowned upon...teams liked the PRs to be one logical unit/fix/improvement and component parts became frustrating or got merged at different times, creating the need for rework.
From reading a lot of feedback on this post one thing that stands out is, the best way to use git just depends on the context. But it doesn't hurt to have commands like this in your toolbox and know how to use the tool well. Plus we all have our private, icky antipatterns that we know we should improve, right?
I dislike the rule of "small PRs" because people tend to interpret it as "arbitrary number of lines changed", and end up breaking a bugfix or feature into many PRs that don't make sense to review or release in isolation. I interpret "small PRs" as being "PRs should be about one bugfix/feature".
I use lazygit[0] to essentially do the same thing. (or even vim-fugitive[1])
Previously I avoided creating messy commits simply because it was "tedious" to reorganize commits. And making overly atomic commits and typing out git commands more frequently didn't appeal to me either.
Once I got used to the above tools, life got so much easier. Lazygit makes it super easy to amend, reword, and even re-position commits in a TUI environment. Real life-changer for me. I can stage hunks super easily too, though that's even easier in neovim.
The only issue I face is my C-j/C-k keys are already bound to tmux, but are needed by lazygit to reposition commits.
The use of `git reset` is a little silly and is playing with fire for no absolutely no benefit.
If you want to follow the pattern described here, create a new branch and do `git checkout the_feature_branch -- $filename` and pull the changes from the branch you did the work in into the new branch, making commits as described in the article. You can even do `git diff --numstat $feature_branch $new_branch` to see what has changed.
Trying to understand where the danger is with git reset. To avoid a very harmless and easily reversible command, you complicated the process with another branch. The fact you feel it is dangerous tells me that you don't have a strong understanding of how git works. It's always kind of funny to me to see how strong of an opinion devs get over how to use git when it's clear they don't really understand the toolset. It took a while to get all the devs at my last company to understand how simple it is to rebase/reset, and write some publish worthy commits, but once they all understood the value it adds, they all became converts and did the evangelism job for me the next time we hired a dev. I recommend reading Pro Git by Scott Chacon. Git is pretty dang amazing.
The reset part seem superfluous, most of this can be handled via rebasing. In fact, this is exactly what we do at <day job>.
Much like the thesis of this article, the goal is to have a set of well organized commits so that when it comes time to do a PR review, you have 1-3 logical units of change and it also improves the ability to do reverts.
But you can very easily do this just by rebasing instead of resetting and re-committing.
In particular, `git commit --fixup <ref>` and `git commit --squash <ref>` are extremely useful for this during the "WIP" stage as well as when handling PR comments, and these are things that I only learned about in the last 6 months. I would recommend doing a google search on "fixup commits" to learn more about them. I enjoyed this article: https://www.mikulskibartosz.name/git-fixup-explained/
Yes, rebasing is scary if you're new to git, but it does everything in this article in a way that's much cleaner and once you learn how to use it effectively you'll feel like you have super powers. It's worth taking the time to learn.
Right, I don't think rebasing is that daunting if you take a few hours to practice. It's time well spent.
Sometimes you can even create empty commits (--allow-empty) up-front (if you already know about all the things that have to be changed) and fill them via fixup/rebase --autosquash
After learning about fixup and squash commits, I started typing `git rebase -i --autosquash` so frequently that I made an alias for it. It has totally changed the way that I work. You get to have the best of both worlds; commit early, commit often but then also having nice and tidy PR's and histories.
The one thing I wish were easier would be breaking a single commit into two commits.
Edit: to be clear, I know it's possible by using git reset... but I'd much rather have something like git add -i that interactively shifts changes from the commit itself and moves them into working directory-only changes.
1. `git rebase -i` to the commit before the one you want to split
2. mark the commit you want to split as an `edit`
3. remove the file(s) you want to edit from the index so that it's part of your working tree
4. use `git add -p` to stage the hunks you want in the first commit (assuming you want to split commits in a single file) or just commit the files you want in the first commit first, second commit second.
VS Code also makes it easy to stage hunks in the editor if you like to stay in your editor; in the diff viewer from the Git view you can highlight the lines you want to stage and right click -> stage selected ranges.
Give it a try and compare. tig lets you either stage hunks ('u') or lines ('1'). You can also adjust the context ('[', ']') and split chunks ('\') and revert chunks ('!') all from the same view.
+1 for tig, and for the folks that are less TUI-inclined, there's `git gui`; a neat little tool which is far too little known considering that it's part of core Git. One of the nice things about `git gui` is that it has an "amend last commit" mode that makes splitting up a commit trivial (unless the commit also adds or removes files -- in that case, you're going to have to do some more work).
Rebase is great, but it's usefulness really depends on the commit history.
If your branch consists of 10 commits of trying various things, then the sum of changes in all the commits can easily become significantly larger than the final branch diff. In that case resetting is orders of magnitudes faster than fiddling with rebase.
That's the point of squashing commits - Many of your changes will ultimately pare down into just a few logical thoughts, adding function X to feature A, then using that in function Y in feature B etc... It's not something that comes for free, of course, but if you make many small commits and try to get each file to compile each time you can often basically just squash by file or directory.
I would reject a merge from a branch that does more than one thing at a time as you've described during code review. It's okay to have a branch that does a bunch of things at the same time, but come time to merge back to `main` this should be broken up in to several different PR's.
I would probably branch off my branch for each logical change I wanted merged and use `rebase -i` to wholesale drop commits that I didn't want on the branch anymore.
This is also an example of where stacked diffs are far superior to PR's but that's a separate issue.
Nothing in sandgiant's post implied that there's more than one thing happening in the branch. Quite the opposite actually, they said that the final diff could be much smaller than the sum of all the small WIP commits along the way. It could be that after hours of digging, you found the real bug was a typo somewhere, and the final diff may be just a 2-line change, where one of the WIP commits was rewriting the whole auth layer because you were chasing a dead-end.
Even with autosquash, rebasing has to iterate through each commit individually before creating the final commit. Which means it's touching a bunch of files in your working tree while that's happening, which means build tools like make now have to rebuild them because the timestamps have changed.
What's worse, is if you rebase (say, 20 commits) onto a newer branch than the one you started with, and you have merge conflicts (say, on commit #10), now you might have to fix the conflicts over and over for every commit after the first conflicting one (depending on the contents of the commits). I know there's `git rerere`, but after 16 years using git I've still never learned how it works.
Using reset to squash first, guarantees that you create your single commit first, without dealing with merge conflicts/etc, and then you can rebase that resulting commit once, to deal with conflicts once.
Fix-ups are nice. I developed a script that takes the all staged content and breaks them up into separate fix-ups that fixup the last commit that changed those lines. I find it pretty handy
maybe, but it's probably much simpler. It's just a small bash script that runs git blame twice for each line in the index and creates a fixup per commit found
Yes, rebase is awesome and every Git user should learn about it. But why not both?
When I'm working on a more complex patch series, it's not uncommon for me to have a bunch of already cleaned up, or semi-cleaned up commits on top of the remote branch I'm working against, and then a bunch of random garbage "WIP" commits. Something that often happens is that I `git reset` not to the upstream branch but to my most recent clean commit, take the resulting changes apart into individual commits (often, some of them will be squash/fixup as I discovered a bug or missing piece of earlier work) and then `git rebase` with `rebase.autosquash` enabled in my global Git config.
Sure. I do that as well. But this article was advocating for using a soft reset to replicate what is ostensibly the built-in fixup mechanisms from interactive rebasing.
As far as I could tell, this was the recommended workflow from the earliest users of git. Look at the Linux kernel's public history.
When git gained widespread adoption, it was sold as "Subversion, but with cheap branching." People don't realize that this power requires discipline, otherwise you end up with complicated history that makes change management a nightmare.
In code review systems like Gerrit, you have to approve every single commit, and you can also enforce rules like "fast-forward merges only." It sure makes Github's pull-request model feel like an anti pattern.
> Treat yourself as a writer and approach each commit as a chapter in a book. Writers don't publish first drafts. Michael Crichton said, "Great books aren't written-- they're rewritten."
This is a more succinct and poignant way of describing my workflow, and why "just put a PR up with all your WIP commits" is so difficult for me. I don't want people to see how long I spent getting this one unit test to pass when the answer was staring me in the face the whole time. I don't people to see just how badly I wrote the first draft of my code, when my approach was basically "make it work e2e first, no matter how bad of a hack it is, then actually figure out how the code should be laid out later."
Folks say "your PR can just be squashed when merging anyway", but it misses the point: My private commit history is very, very, intensely personal to me, and I don't want anybody seeing it, ever.
I highly recommend tig because it lets you easily stage each line/change separately. Together with interactive rebase and fixup commits, it becomes super easy to group changes where they belong. For fixup commits with tig (in main view go to the commit to fixup and press '='), my .gitconfig has:
[tig "bind"]
main = = !git commit --fixup=%(commit)
The article starts by stating a rather obvious problem: if you revert a commit, you might break something because any commit following the reverted one could build on what was added in the reverted commit.
Then the article describes a very elaborate way of… not addressing the problem?
Reset-Cleanup is a decent idea and helps keep a tidy repo which is a good goal. But the revert reason seems contrived.
The problem is going to be later code (from other pull requests) being dependent on the code in my pull request.
If my pull request is reverted then any later change can break. If my PR consists of 5 well separated commits or 3 messsy ones doesn’t matter in that scenario.
If someone against all odds wants to revert one individual commit from a merged branch but not revert the whole PR by reverting the merge commit then not only can the feature in the PR stop working (if the feature/bug fix in the PR didn’t need that commit to work then why was it there in the first place!?), any later code can break just as it can when PR is reverted as a whole.
This is why I recommend squashing for almost all cases. For really complex features with dozens of commits you can always do a rebase + FF (but in that case all commits should build + pass tests, which is an unrealistic goal in all code bases where a build + test takes hours).
Most folks are complaining about `git reset` being more dangerous and to just use `rebase -i` instead. To each their own... I'm one of those weirdos who also uses the `git reset` in their workflow. I prefer it to `rebase -i` because it doesn't mess with my working tree a bunch while it's happening, plus I like to construct brand new logical commits, in order, in the manner described by the article.
But this part set off alarm bells:
> Once you’ve finished making your changes, it’s time to prepare your work for some “git clean up.” To do this, we’ll run the following command:
> git reset origin/main
Be very careful here! If you've run `git fetch origin` since you've started your work, you may be resetting to a commit that's newer than what you based your work on, and thus you'll wind up creating a commit which effectively reverts anything that's happened on `main` since then.
The more technically correct command would be:
> git reset $(git merge-base origin/main HEAD)
Since that resets to the commit you started your branch from.
Yes, this suggestion effectively addresses the biggest problem with the flow described in the linked post. Accidentally clobbering changes in the main branch used to be one of the top sources of bugs when I started at my current job (incidentally, switching to a rebase workflow helped with this, though I'm sure that wasn't the only route we could have taken).
the only gotcha with rebase is that it cherry picks commits and so if you do it willy nilly then you can end up introducing changes where you don't expect them and reverting reverts
it's more complicated but vastly more powerful than reset
My problem with rebasing is that the vast majority of my commits are garbage "WIP" commits, and I want to squash them away anyway.
Squash-rebasing on top of main makes git replay all of those dumb commits, just to squash them, and I have to fix merge conflicts individually, for every commit I don't even care about, before it's done.
I'd much rather soft-reset to the merge-base, make one clean commit, then rebase that one commit, than any of the other approaches.
The typical response people have is that I shouldn't create so many garbage WIP commits, but... that's just not how my brain works. See https://news.ycombinator.com/item?id=30000320
Why do we keep trying to make git "smart"er than it needs to be? Whenever I hear someone complain about git it's 9/10 times because of ignorance, and it's a simple mistake to solve. No amount of tooling or smartening (as this author seemed to be described) is a valid alternative to training and understanding. The consequences of that is that a smarter tool used by dumb people means those people are held back, and made unproductive by the tool in question.
The article isn't proposing to change anything about git, it just shows one way of cleaning up your branches before merging them... which I guess makes people understand git better? The article is nothing groundbreaking, it's just showing how to use git reset, but still.
This article undermines itself the second it moves from “here’s the problem” to “here’s the solution:”
> Be mindful of not leaving your codebase in a broken state during this step
You’re still relying on very fallible human intervention here. Even worse, often times when grouping commits post-facto you’ve forgotten some of the context of what depends on what.
Ostensibly the approach presented could be better than other strategies in this regard, but that’s now how the article presents itself and it loses credibility for that.
I'm surprised no one has mentioned trunk based development yet. Never going back to git flow at a medium/large org again. Merge hell is a nightmare and releases more so.
> When the feature is complete, make a pull request.
gotta love 100 file commits that take a day to review.
I hate Perforce (most because of bugs) BUT the in progress CL workflow is better than this.
Git should introduce stages and allow you to have any number of stages. Git should also have some porcelain around stashing all but one stage and restoring all etc etc.
I hate to see git guides where they tell the user to make commits that are literally the description of the diff rather than the why of the change.
"Added new styles to navigation"
No duh, that's what the diff shows, but that doesn't tell us why you made the change, and that's the part we're going to struggle to remember in 6 months.
I feel that these "workflow suggestions" are all nice and dandy, but have nothing to do with the real world. The real world looks like this: Create a new branch, make changes, commit, make a PR, commit some more (all without proper commit messages of course), make more fixup commits, and when it gets merged it gets all squashed into one big commit, rebased on master and ff-merged.
This is what the project I was just hired to work on looks like. Result is a linear history, with multi-thousand-line changes in a commit with message "Implement fixes" and a list of "fix bug", "fix more", "format", "change implementation" in the long-form commit message.
People out there do not even know how to work properly with git, I think it is way too much for them when we start telling them how to "workflow" with git. It is sad, but that's what I see.
114 comments
[ 5.1 ms ] story [ 159 ms ] threadAll depends on if you want the most efficient approach, or if you want serendipity to strike.
Depends on what you want to revert? git revert accepts more than one commit!
I'm not sure this gives the right impression. Yes rename/squash/interactive rebase if necessary to tidy, however I still believe you should strive to create clear, separate commits as you go.
If you have to regularly make major changes to history before review, I could be a sign that your process/approach is disorganised.
Right. Next time think before do.
Of course my process is disorganized! I'm an extremely disorganized person. I like to try out wild tangents in my coding, experimenting with some crazy idea or another and saving my place before and after I do so. I even have a separate .txt file of stream-of-consciousness writing about my coding, to keep track of the tremendous amount of complexity I have to deal with on a regular basis.
My reflog reads like a personal diary of failings, dead-ends, "FINALLY COMPILES!" commits, etc etc. I use `git branch someNewBranch` really often, keeping a namespace of local branches under the name `deadend/*` to mark tombstones of approaches I tried in code but didn't work out, and I `reset --hard` back to a previous commit after I save the branch. I like it this way, it maps better to my totally disorganized brain.
But I'm also a firm believer that none of my colleagues should be able to tell how disorganized all of this is, because by the time I make a PR, I squash it all down and write a very long, detailed commit message of what exactly I did, why I'm doing it, and how it works (sometimes, albeit rarely, spread across multiple logical commits in one PR.) They never get to see my private commit history.
Why does it matter to anyone how much I have to prune my commit history before a PR? That's like complaining to another student in class that their short-form handwriting is hard to read, in their own personal notes they're taking during class, when said student is acing all the tests. It's simply not the metric you should be judging people on.
Either way, in the end it's up to developer discipline to make clean commits, git doesn't really help all that much although in my (long) source control history before git I never thought of committing hunks.
Because committing is useful? It saves my work. I can get to a point where my code finally compiles, make a quick commit, then do some more dangerous refactoring. If something breaks, I can `git diff` to see what changes I've made since it last compiled.
I'll turn this around and ask you: Why do you feel like you can only make a commit if you have something that's reviewable? Is your code writing style so perfect that it all works the first time? Or do you just not bother committing anything until your work is done? (That personally sounds terrifying to me... I really need to be able to say "what have I changed since 10 minutes ago when this compiled?" on a very regular basis.)
A typical workflow for me:
Contrast this with using `rebase -i` to squash: They both feel like the same amount of typing to me, but `rebase -i` thrashes my working tree around with each commit, which has a habit of confusing my IDE quite a bit and breaking its build cache. Also, I like the `git diff` step in the reset workflow because it gives me a nice reminder of what my change looks like, which helps inform how I should word my commit. So I use reset.If I want to keep my intermediate work locally for posterity, I usually use `git branch prprep/<bug_number>_unsquashed` or something so that I can dig it up later if need be. I don't push any of my `prprep/` branches.
Bingo. I do this all. the. time. I don't want anybody seeing it. I honestly couldn't imagine another way of working... commit early, commit often, I say. Hell, commit on a timer every minute, if that's your thing (I've never done this but totally understand people who do.) Leave the meaningful messages for later when you're actually ready to craft a PR.
Honest question: if you avoid committing code unless it's working and you have a meaningful message to make, what happens if you screw up your editor undo, and you need to go back to where you were 10 minutes ago? This happens all the time for me... I undo a whole bunch and then accidentally type the "z" button into my buffer because I fumbled the keyboard, and now I can't "redo" back again. If I commit early and often, I can recover the work to where it was, even if it's not compiling yet. If I have to wait for my code to be basically "reviewable", I'd be screwed.
And the WIP commits can still be done, but locally. Then I clean them up everytime I push to the remote branch, but not when that branch is merged.
It occurs to me that people like me might actually be quite rare, who use git almost entirely locally and rarely push. The only times I push are when creating a PR (hence all the resetting and rewriting first), or to back up my WIP work, if it's important. And even then it's to my own fork, and under a branch namespace that nobody would ever confuse with real code that should be reviewed.
When developing my priority is to easily get back to a last known working state. This allows me to try out risky changes, and throw them all away if it doesn't work out.
When pushing my changes for PR, those working states I saved before may not be the best logically. I usually re-write my commits to break it into more logical chunks, that are easy to revert and easier for other teammates to digest.
I don't care one lick what someone's branch history looks like. If they want to commit every day, or every hour, or after every keystroke - I don't care. All I know is that once the PR is merged, it's all going to be squashed into a logical unit so the `main` commit history will look just fine.
Dont make work upfront, that is rarely important. When it is important, find the commit and split it up (as necessary) at that singular point (instead of across all PRs). You have now cut down how much time it takes to make PRs while retaining the same end result.
Spending effort on managing git is mental effort you don't spend on solving your actual problems. By far the best experience I've ever hadeith git was: everyone works straight on the dev branch, just rebase, fix your stuff, test often, and if you're doing some multi-day work then sure, branch and think it over then merge.
That's it. That's all you need. I've had a million more problems with every attempt at making this process "clean", or "smart". Dumb was by far more efficient, more enjoyable, helped us find and fix bugs faster, and had the shortest time to market ever.
The advice I try to live by, is that however messy my work was leading up to a PR, I make sure the end result is something somebody can review without additional context. Commits should have lengthy descriptions of changes that describe the "why" and "how" of a particular change, in a way that makes it easy to digest for a reviewer. Sometimes multiple commits make sense (like if you're renaming a module/class, put that in a single commit, then put the actual code change in the next one), sometimes they're not necessary. But it's worth it to put in the effort here if it means it helps a reviewer, IMO.
I could avoid creating those commits in the first place, but asking me to only commit my local changes when I have something intelligent to say about them, is a damn near impossibility for me. I basically use git to "save my work" before I try an approach to something. I reset back if it doesn't work out. Sometimes I reset back again, if the approach that didn't work out turned out to be the least worst option. I create commits to experiment with something, so that I can quickly compare back and forth between two approaches. etc. etc. etc.
I would instead say, that if you're only making commits when you have something logical to say, you're probably not using git to its fullest potential. You should really go nuts with it IMO, and only bother to sound intelligent in your commit messages once you're ready to do so, and (most importantly) you should still make commits before that happens. Git's decentralized for a reason, take advantage!
Nothing forces you to keep the messy commit messages either - keep the short commit message and make sure it's good, then delete the combined individual commit messages from the long message field, done.
About as easy as it gets. As titles are issue numbers changes are documented automatically.
Yes, that probably is a “me” problem, and I’m too anxious about it for no reason, but that’s me. But if you saw my local scratch branches you’d probably understand.
(My wip commits occasionally bitch about coworkers, for example. Or they just contain a bit too much profanity for a professional environment. Or there’s just 50 worthless “WIP” commits with no other description. You’d never know any of this from my PRs.)
That will solve your problems if you're the messy type.
You said it’s a waste of time to do this and to just squash when merging instead. I’m saying I’d rather squash first so that my PR looks clean and doesn’t contain my WIP commits. Then you respond saying “well you can just squash before your PR then”… are we going in circles?
My original comment was targeting the two supposed problems that the article is framing and the supposed convoluted (and possibly dangerous) solution with git reset.
If someone else is doing the merge and you’re unsure if they will you can preemptively squash your whole branch so there’s only one commit in the pr.
I agree! And that's what I do. But OP said that would be "a waste of time and effort", hence my reply.
besides CI only needs to test the merge commit
to me, squashing the tree to simplify an edge case seems needlessly radical
I assume people here have different workflows, where they work alone on small features for maybe 1-2 days and then just squash all their tiny WIP commits into one?
Yes, that's true. My original point still stands though. That commit will have a PR associated with it. You can go to it and see the details of all commits that were used for the merge commit that went into master. Not really a lot of extra work to go to the PR if you're investigating that deep.
> Even if the original branch is still floating around somewhere, that would still be an extra hassle.
If the branch deletion setting when merging a PR is selected, then only the origin branch will be deleted. If you're the one doing the PR and you don't delete your local branches you'll be able to see it locally. In the other case where is someone else's then it becomes harder, and I agree it's an extra hassle.
In my work I try to mandate PR's to be as small as possible, but not smaller. Big PR's do happen, but that and resorting to git blame or bisect happens maybe 2-3 times a year. I think the general rules for source control followed by the team are more important than the implementation details and git magic.
The issue is one of review scaling. I wrote a blog post about this a while ago[0], but the gist of it is that those clean logical units are often too small for meaningful high-level reviews of more complex work.
With complex features or refactorings, you're often in a situation where those clean logical units allow reviewers to do a good low-level review (do a check for logic corner cases, style issues, etc.) but they don´t allow a high-level review of how all the pieces of the feature work together.
IMHO the most open-source process friendly solution to the issue is to review patch series, where you can review the series as a whole for the big picture, but also dig into individual commits for the details. Building such a patch series requires an approach as described in the article.
(In closed source environments, you may get a good enough approximation of the result with a separate, disciplined software design process.)
[0] http://nhaehnle.blogspot.com/2020/06/they-want-to-be-small-t...
For example,
Most of one core change is in multiple files, it will be very bad to commit by files group
I use GitHub and always squash commits before merging a PR. This keeps the commit log clean and also has the side effect that you have the PR number in the merge commit.
Having said that I also suggest keeping PRs small. If you are going to reformat a code base, make that a separate commit. Updating a library, separate commit. Adding a library you need for a new feature: create a PR for the library update, base your implementation off that branch and rebase against main when the first PR is merged.
From reading a lot of feedback on this post one thing that stands out is, the best way to use git just depends on the context. But it doesn't hurt to have commands like this in your toolbox and know how to use the tool well. Plus we all have our private, icky antipatterns that we know we should improve, right?
Previously I avoided creating messy commits simply because it was "tedious" to reorganize commits. And making overly atomic commits and typing out git commands more frequently didn't appeal to me either.
Once I got used to the above tools, life got so much easier. Lazygit makes it super easy to amend, reword, and even re-position commits in a TUI environment. Real life-changer for me. I can stage hunks super easily too, though that's even easier in neovim.
The only issue I face is my C-j/C-k keys are already bound to tmux, but are needed by lazygit to reposition commits.
[0]: https://github.com/jesseduffield/lazygit [1]: https://github.com/tpope/vim-fugitive
If you want to follow the pattern described here, create a new branch and do `git checkout the_feature_branch -- $filename` and pull the changes from the branch you did the work in into the new branch, making commits as described in the article. You can even do `git diff --numstat $feature_branch $new_branch` to see what has changed.
How so? The previous commits are still recoverable from the reflog.
the problem with your approach is that sometimes, changes are logically grouped but cross file.
Much like the thesis of this article, the goal is to have a set of well organized commits so that when it comes time to do a PR review, you have 1-3 logical units of change and it also improves the ability to do reverts.
But you can very easily do this just by rebasing instead of resetting and re-committing.
In particular, `git commit --fixup <ref>` and `git commit --squash <ref>` are extremely useful for this during the "WIP" stage as well as when handling PR comments, and these are things that I only learned about in the last 6 months. I would recommend doing a google search on "fixup commits" to learn more about them. I enjoyed this article: https://www.mikulskibartosz.name/git-fixup-explained/
Yes, rebasing is scary if you're new to git, but it does everything in this article in a way that's much cleaner and once you learn how to use it effectively you'll feel like you have super powers. It's worth taking the time to learn.
Sometimes you can even create empty commits (--allow-empty) up-front (if you already know about all the things that have to be changed) and fill them via fixup/rebase --autosquash
Edit: to be clear, I know it's possible by using git reset... but I'd much rather have something like git add -i that interactively shifts changes from the commit itself and moves them into working directory-only changes.
2. mark the commit you want to split as an `edit`
3. remove the file(s) you want to edit from the index so that it's part of your working tree
4. use `git add -p` to stage the hunks you want in the first commit (assuming you want to split commits in a single file) or just commit the files you want in the first commit first, second commit second.
I strongly recommend tig
VS Code also makes it easy to stage hunks in the editor if you like to stay in your editor; in the diff viewer from the Git view you can highlight the lines you want to stage and right click -> stage selected ranges.
I've never used tig. I'll look at it.
If your branch consists of 10 commits of trying various things, then the sum of changes in all the commits can easily become significantly larger than the final branch diff. In that case resetting is orders of magnitudes faster than fiddling with rebase.
Use fixup commits.
I would probably branch off my branch for each logical change I wanted merged and use `rebase -i` to wholesale drop commits that I didn't want on the branch anymore.
This is also an example of where stacked diffs are far superior to PR's but that's a separate issue.
What's worse, is if you rebase (say, 20 commits) onto a newer branch than the one you started with, and you have merge conflicts (say, on commit #10), now you might have to fix the conflicts over and over for every commit after the first conflicting one (depending on the contents of the commits). I know there's `git rerere`, but after 16 years using git I've still never learned how it works.
Using reset to squash first, guarantees that you create your single commit first, without dealing with merge conflicts/etc, and then you can rebase that resulting commit once, to deal with conflicts once.
maybe, but it's probably much simpler. It's just a small bash script that runs git blame twice for each line in the index and creates a fixup per commit found
When I'm working on a more complex patch series, it's not uncommon for me to have a bunch of already cleaned up, or semi-cleaned up commits on top of the remote branch I'm working against, and then a bunch of random garbage "WIP" commits. Something that often happens is that I `git reset` not to the upstream branch but to my most recent clean commit, take the resulting changes apart into individual commits (often, some of them will be squash/fixup as I discovered a bug or missing piece of earlier work) and then `git rebase` with `rebase.autosquash` enabled in my global Git config.
As far as I could tell, this was the recommended workflow from the earliest users of git. Look at the Linux kernel's public history.
When git gained widespread adoption, it was sold as "Subversion, but with cheap branching." People don't realize that this power requires discipline, otherwise you end up with complicated history that makes change management a nightmare.
In code review systems like Gerrit, you have to approve every single commit, and you can also enforce rules like "fast-forward merges only." It sure makes Github's pull-request model feel like an anti pattern.
> Treat yourself as a writer and approach each commit as a chapter in a book. Writers don't publish first drafts. Michael Crichton said, "Great books aren't written-- they're rewritten."
This is a more succinct and poignant way of describing my workflow, and why "just put a PR up with all your WIP commits" is so difficult for me. I don't want people to see how long I spent getting this one unit test to pass when the answer was staring me in the face the whole time. I don't people to see just how badly I wrote the first draft of my code, when my approach was basically "make it work e2e first, no matter how bad of a hack it is, then actually figure out how the code should be laid out later."
Folks say "your PR can just be squashed when merging anyway", but it misses the point: My private commit history is very, very, intensely personal to me, and I don't want anybody seeing it, ever.
Example from the start ... revert.
Don't revert stuff, fix stuff forward (make new commits with new changes) you won't introduce bugs in silly way.
Then the article describes a very elaborate way of… not addressing the problem?
Reset-Cleanup is a decent idea and helps keep a tidy repo which is a good goal. But the revert reason seems contrived.
The problem is going to be later code (from other pull requests) being dependent on the code in my pull request.
If my pull request is reverted then any later change can break. If my PR consists of 5 well separated commits or 3 messsy ones doesn’t matter in that scenario.
If someone against all odds wants to revert one individual commit from a merged branch but not revert the whole PR by reverting the merge commit then not only can the feature in the PR stop working (if the feature/bug fix in the PR didn’t need that commit to work then why was it there in the first place!?), any later code can break just as it can when PR is reverted as a whole.
This is why I recommend squashing for almost all cases. For really complex features with dozens of commits you can always do a rebase + FF (but in that case all commits should build + pass tests, which is an unrealistic goal in all code bases where a build + test takes hours).
But this part set off alarm bells:
> Once you’ve finished making your changes, it’s time to prepare your work for some “git clean up.” To do this, we’ll run the following command:
> git reset origin/main
Be very careful here! If you've run `git fetch origin` since you've started your work, you may be resetting to a commit that's newer than what you based your work on, and thus you'll wind up creating a commit which effectively reverts anything that's happened on `main` since then.
The more technically correct command would be:
> git reset $(git merge-base origin/main HEAD)
Since that resets to the commit you started your branch from.
it's more complicated but vastly more powerful than reset
Squash-rebasing on top of main makes git replay all of those dumb commits, just to squash them, and I have to fix merge conflicts individually, for every commit I don't even care about, before it's done.
I'd much rather soft-reset to the merge-base, make one clean commit, then rebase that one commit, than any of the other approaches.
The typical response people have is that I shouldn't create so many garbage WIP commits, but... that's just not how my brain works. See https://news.ycombinator.com/item?id=30000320
> Be mindful of not leaving your codebase in a broken state during this step
You’re still relying on very fallible human intervention here. Even worse, often times when grouping commits post-facto you’ve forgotten some of the context of what depends on what.
Ostensibly the approach presented could be better than other strategies in this regard, but that’s now how the article presents itself and it loses credibility for that.
> When the feature is complete, make a pull request.
gotta love 100 file commits that take a day to review.
Git should introduce stages and allow you to have any number of stages. Git should also have some porcelain around stashing all but one stage and restoring all etc etc.
"Added new styles to navigation"
No duh, that's what the diff shows, but that doesn't tell us why you made the change, and that's the part we're going to struggle to remember in 6 months.
This is what the project I was just hired to work on looks like. Result is a linear history, with multi-thousand-line changes in a commit with message "Implement fixes" and a list of "fix bug", "fix more", "format", "change implementation" in the long-form commit message.
People out there do not even know how to work properly with git, I think it is way too much for them when we start telling them how to "workflow" with git. It is sad, but that's what I see.