13 ms·
A Better Git Flow
- mattwad 5y agoMy 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.
- derekp7 5y agoWhen 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.
- okl 5y ago> easy to revert later Depends on what you want to revert? git revert accepts more than one commit!
- leni536 5y agoYou can even revert a merge commit, if you specify the parent.
- agd 5y ago> do the work first, clean up the commits later 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.
- okl 5y ago> 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.
- ninkendo 5y ago> 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.
- zwieback 5y agoWhy 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.
- ninkendo 5y ago> Why commit at all if you're going to reset? 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.)
- zwieback 5y agoOh, 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.
- ninkendo 5y agoThe 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.
- joelmbell 5y agoI 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.
- _hao 5y agoWhat 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.
- mttjj 5y agoBingo! 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.
- Supermancho 5y agoaka Premature process optimization. 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.
- jcranmer 5y agoThe 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.
- laserbeam 5y agoThere'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.
- mb20281 5y agouhh...git rebase -i anyone?
- pwdisswordfish9 5y agoYeah, this basically how git was supposed to be used to begin with.
- timmit 5y agoIt makes sense sometime, but not always. Group commits by files. For example, Most of one core change is in multiple files, it will be very bad to commit by files group
- okl 5y agoI disagree. It's not difficult to stage single lines using the right tool.
- leifg 5y agoThis 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.
- dnilasor 5y ago+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?
- hcarvalhoalves 5y agoI 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".
- okl 5y agoSquashing commits is something lazy people do! :P
- bronzecarnage 5y agoI 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. [0]: https://github.com/jesseduffield/lazygit https://github.com/jesseduffield/lazygit [1]: https://github.com/tpope/vim-fugitive https://github.com/tpope/vim-fugitive
- ltbarcly3 5y agoThe 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.
- Rolcol 5y ago> and is playing with fire for no absolutely no benefit How so? The previous commits are still recoverable from the reflog.
- deadbunny 5y agoI've found most people can barely use git, yet alone fucking with reflog
- geoffmanning 5y agoTrying 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.
- whoomp12342 5y agoOr you can just use staging to bunch up your changes until you are ready for a logical commit.... the problem with your approach is that sometimes, changes are logically grouped but cross file.
- dljsjr 5y agoThe 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/ 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.
- okl 5y agoRight, 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
- dljsjr 5y agoAfter 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.
- okl 5y agoMy aliases: alias.c commit alias.cf commit --fixup alias.cm commit --message alias.co checkout alias.f fetch -v -p alias.par pull --rebase --autostash alias.ri rebase --interactive --autosquash --autostash alias.rim rebase --interactive --autosquash --autostash master alias.s status alias.l !git --no-pager log --oneline -n10 alias.pf push --force-with-lease alias.bs !git --no-pager diff --stat master..HEAD alias.p push alias.alias !git --no-pager config --get-regexp alias Sometimes I get the feeling that my job isn't programming but managing text snippets (diffs).
- sandofsky 5y agoI wrote a similar suggestion in 2011: https://sandofsky.com/workflow/git-workflow/ https://sandofsky.com/workflow/git-workflow/ 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.
- ninkendo 5y agoI love this quote: > 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.
- okl 5y agoI 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)
- hill613 5y agoAnd if you have PR comments/commits you do this everytime? Just squash
- ozim 5y agoYes git rebase -i is better as others pointed out. 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.
- alkonaut 5y agoThe 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).
- ninkendo 5y agoMost 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.
- iamed2 5y agoYes, 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).
- cerved 5y agothe 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
- ninkendo 5y agoMy 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 https://news.ycombinator.com/item?id=30000320
- danhab99 5y agoWhy 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.
- Double_a_92 5y agoThe 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.
- travisd 5y agoThis 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.
- alunchbox 5y agoI'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.
- jayd16 5y agoI 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.
- krainboltgreene 5y agoI 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.
- endigma 5y agoI always thought this was how git commits were meant to work? Isn't this just GitHub flow with sensible commit grouping?
- musicmatze 5y agoI 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.
- lamontcg 5y agoThis doesn't really have anything to do with the Git Flow model which is all about having multiple long-lived development branches. As far as the proposal goes, this is a much better way to do it than trying to preserve every mistake you made along the way. I'd prefer to see either this or just the one squashed commit. I'd just argue that maybe if its important to break it up into multiple commits that you might consider that it should also be separate PRs. If it is necessarily so interlinked that you can't produce separate passing PRs then I probably need to wrap my brain around the whole thing at once as well. Usually my objection is that multiple different concerns need to be done totally separately. I can't think of the last time I really wanted just separate commits and not entirely different PRs.