I used to do this but it would almost always get messy. If changes are necessary after code review on a PR early in the chain (and you should assume there will be), you need to rebase or merge the earlier one into any PRs that depend on it. It’s often not possible to do this cleanly.
Nope. Hard disagree. You need to rebase. Only rebase, no merge, never merge in this situation. It just fucks up the tree and make hard to figure out what should go where. I only ever merge in one direction, and rebase in the other.
I actually 100% agree, and would only ever rebase in this situation. I only added “or merge” in anticipation of some people who get weird about rebasing anything ever. :P
git-branchless handles this well (author here). You just check out the old commit and amend it and run git restack. This updates all descendant commits and branches. It won’t start merge conflict resolution unless you pass --merge, so you can decide to resolve conflicts either now or later. You can also use git-branchless’s amend tool to do both in one step.
You literally can’t do this with vanilla git or you’ll go crazy. Ben was underselling how hard it is in his post.
I currently use git-branchless thanks to @arxanas hard work. The only thing it’s missing is the ability to sync a branch of pull requests up to Bit Bucket but I’m not expect any tool to get that anytime soon ;(
Interesting, I haven’t noticed this in my experience. But I definitely see the risk with stacking commits that fundamental changes in the base commits would invalidate assumptions used to build later ones
Not sure I follow, but if the early commit has already been merged, and then you realize you have to make a fix before the next one goes in, then those just rebase on the fix – why is that a problem? What is the alternative?
The problem is with thrashing between several PRs and keeping track of which ones need updating and in what order (i.e. if you have a dep chain of 3+ PRs). The other problem is dealing with conflicts, which is just annoying. In my experience you’re just trading one kind of mental overhead for another.
The alternative is to just work on something else while the base PR is under review. IMO this is a social/collaboration problem, not a technical one. :)
I would say the solution is small batch sizes and a fast enough PR turnaround time. If you have those things, you can keep working on the next dependent PRs, and even if the reviewer requests larger changes than you anticipated, the rebasing and conflict resolution work should never be unmanageable.
Said another way, batch sizes are too big, or reviews happen too slowly, if the ripple effects of review scare you to the point of pausing work.
Well yeah, you’d need to rebase one, merge it, then the next, etc. My comment above about the social aspect is relevant here: everything will be fine if your PRs are small and getting reviewed quickly. If that’s not happening, you’re hosed anyway. This isn’t a tech problem, and you don’t need these other services on top of git to solve it.
I’m not sure I understand your comment. I think we might be talking about different things. If your version control system doesn’t support you continuing to work by building up changes on top of changes already being reviewed you will not create small, granular code reviews. It’s not a social problem that I’m talking about, it’s very much a tooling problem?
Dev A submits small PR X, and dev B begins reviewing.
During review, dev A continues work on Y and Z, branching off still unmerged X.
B requests changes to X
A switches back to X, makes changes, pushes them.
A rebases Y and Z on X.
B approves and merges X into main.
A rebases Y and Z on main, and now submits Y as a new PR.
etc…
My claim is that this works perfectly fine in vanilla git, and rebasing is quite easy, so long as we have small PRs moving along quickly. I guess my question to you is: If you disagree with this claim, what is the situation that breaks it and why do I not encounter it? My guess is that the answer would have to be differences in team size or culture, and not tooling.
This is what I’m disagreeing with. This doesn’t work well and requires a lot of manual work.
If you rebase Z on X you bring Y’s commits but not the Y branch pointer. If you rebase Y on X you bring neither the Z commits nor the Z branch pointer. You can force move the branch pointer but this doesn’t scale to a lot of stacked branches, since you’re now doing work PER branch, is generally messy and error prone, and is a pain in the neck. People are often surprised by this behavior but if you go and play with it you’ll quickly see what I mean.
To make stacked branches/PRs work, your vcs must make amending an earlier commit a single command, not one or two commands per descendant branch.
I never have these problem and I do what I decribed almost every day, though tbf I generally am only “1 deep” because the “X” will get merged within a day. But this is what I was saying originally: if your “stack” is so deep you have culture problem. Imo, at that point you change the workflow so you have smaller PRs and a shallow queue.
If you rebase Y on X you bring neither the Z commits nor the Z branch pointer.
Again, this is never a problem in practice for me, because the pain I’ll feel when I later rebase Z only depends on how far the work has drifted / how many conflicts I have to deal with.
but if you go and play with it you’ll quickly see what I mean.
I just played with it in a sample repo and I still don’t get it.
To make stacked branches/PRs work, your vcs must make amending an earlier commit a single command, not one or two commands per descendant branch.
I don’t follow the connection here. In the workflow I use, unless I am cleaning my own history, I am only ever rebasing and fixing conflicts – not amending an earlier commit.
though tbf I generally am only “1 deep” because the “X” will get merged within a day
Your vcs shouldn’t dictate how small your changes can reasonably be or how much time you can take for code review. I sometimes write 5 changes and publish 5 code reviews that build on each other in a single day. Maybe you don’t do that, but it’s not reasonable to say “just get code reviews faster.” git fails fast, small work that builds on earlier work. If that isn’t your workflow (maybe your changes all take 1+ days to do) then that’s cool but it just means you do different work.
And code review can’t really take 1 day for everybody, even if the changes are published slowly. Sometimes code is controversial or hard and needs to have multiple iterations.
I don’t follow the connection here. In the workflow I use, unless I am cleaning my own history, I am only ever rebasing and fixing conflicts – not amending an earlier commit.
The main use case for “cleaning history” is that you get code review feedback and need to amend earlier commits. It’s a pretty straightforward use case.
If git fails for you, fair enough. I don’t claim that there is no situation where vanilla git is not enough, only that I have never encountered it using git regularly for a long time.
The main use case for “cleaning history” is that you get code review feedback and need to amend earlier commits. It’s a pretty straightforward use case.
I still don’t understand your original comment. Is your complaint just that a reviewer asks for a change on a non-head commit in your PR, and in vanilla git you have to rebase -i to do it, so therefore you need another tool to make that easier?
I don’t claim that there is no situation where vanilla git is not enough, only that I have never encountered it using git regularly for a long time.
And that’s totally fair! I’m glad git serves you well. But it sounds like you don’t use stacked PRs, or as you said, your stack is never really more than one branch deep.
Is your complaint just that a reviewer asks for a change on a non-head commit in your PR,
yep!
and in vanilla git you have to rebase -i to do it, so therefore you need another tool to make that easier?
rebase -i cannot handle a stack of branches. It only works on a single branch. If you are rebasing a stack of branches you will not end with the intermediate branch pointers pointing at your rebased code. So therefore I need another tool to make that easier.
If branch A is in code review and I need to amend Commit 2 to make a requested change, there is no way to do this in vanilla git without manually rebasing every stacked branch separately after the amend operation. With other version control systems like Mercurial, no matter how many stacked commits I have the amend is exactly either one or two commands. It scales with the number of stacked commits, or in other words an amend is always O(1) operations, unlike git which is O(# branches). Let me know if that still doesn’t make sense… this comments section is a hard place to explain complicated things.
We also decided to split up feature PRs at work into smaller reviewable units. This is often a bit of a pain when changes needed to be made to the lowest PR in the DAG, since then you need to rebase all the dependent PRs. This will get especially tricky if PRs are squashed on merge. But, having small PRs to review is still a good thing :)
Myself and several co-workers use Graphite. I don’t want to speak for them, but I am a huge fan and would be hard pressed to work in a git repository without such a tool. Not only have my PRs become easier to review, but I’ve felt more confident doing more complex operations in git more easily.
Graphite is an open-source CLI and a code review dashboard built for engineers who want to write and review smaller pull requests, stay unblocked, and ship faster.
Immediately followed by a “join the waitlist” button. How could it be open-source and have a waitlist at the same time?
If I understand correctly, the command-line tool gt is open-source, while the review web service is not. gt is an alternative git-branchless, while the review service is an alternative to GitHub’s UI.
Correct. I essentially never end up using the Graphite Web UI. Its the command line tool gt that I find incredibly useful and would hate to give up. The web UI is good to be sure, but I don’t find it better enough than the GitHub UI to actually reach for it.
I go to the Graphite web UI for one feature: the “merge all” button that automatically deals with GitHub’s braindead approach to reparenting child branches when a parent is merged.
Critical feature: for a reviewer to see the size of a PR at a glance.
If the only thing you know about a review is that you got an email saying that you have a PR pending, you have no way to tell the size except looking at the PR, which is already the biggest hurdle.
That’s why I don’t bother with small PRs at work; they’re easier to review, but they have no way to signal that they’re easier to review, so they have a disadvantage competing for reviewer time.
I use the highly sophisticated solution of putting [easy] in the PR title ;). This works well when most of the commits in the stack are small things like refactoring, and only one or two are substantial.
This is a great point. A couple tools (Gerrit is one of them) display t-shirt sizes of PRs, which I like. (Not sure how good the heuristic is that they use, but I find it easier to judge at a glance between “Small” and “Extra Large” than by looking at diff line count)
What about if someone creates a PR notification service that hits your inbox with all the PRs grouped? Can view in a ‘single PR view’ and also presented as a DAG.
Sounds like the problem isn’t the notification system but that it is overloaded. Email gives you the tools to handle this like filters and subaddressing.
When I received many hundreds of emails a day (admittedly not thousands) my best rule was to prioritize anything that mentioned my actual email, so removing mailing lists from the priority triage. That and unsubscribing from junk was sufficient for dealing with a lot of info on a daily basis. Of course you can always do something more explicit if necessary.
I have a filter which marks emails read when I’m not in the To or CC. This lets me subscribe to high-volume mailing lists without having to sift through everything.
Indeed. I don’t find emails in my inbox, I data mine with gmail search. But seriously, gmail should get good enough with AI to detect and bump up emails it knows you want to read even better than now. I see my gmail will put emails I sent in to the top of my inbox after a few days that need attention to follow up. I indicated I would do just that in the email I had originally sent. I’m terrible at following up. Value add. Conceivable it should know a PR is really important, if you’re obviously using lobste.rs and whatnot. Definitely prefer if all that power wasn’t in Googles hands and some other email service could do that.
Oh I’m not even referring to gmail specifically. At work, where I’m forced to use Outlook, I receive thousands of messages a day where I need to set up almost 100 rules to make email manageable.
I am a big fan of git-branchless. I don’t use it myself as I am quite well versed in git, but it has a lot of optimization that I like and / or wish that I had the time to build myself. Strong recommend from me.
This is really exciting to me. I definitely love the mercurial workflow with a dag of commits that individually get reviewed and submitted. Generally the goal is not to have a deep dag when possible, but overall, it is a much nicer flow than adhoc GitHub branches and PRs.
I wonder how well or poorly this would fit with some open source work I do. I assume it wouldn’t work to be the only person using git-branchless or similar tools. From a first look, it seems to match the workflow I find best, but not at all sure about integration.
I really like the mercurial workflow too, and I’ve been chasing a similar experience in git.
git-branchless is the closest thing I’ve found, but if you need to use branches (e.g. to push PRs to Github), it can be a bit clunky. With Gerrit, git-branchless gets you really close to the mercurial flow
This approach single-handedly leveled up my career when I first started out, it was game changing! What I find is really useful about this is it forces you to think about what you’re going to do before you do it. And that skill becomes incredibly important once you start spending more time planning tasks and less time completing tasks. The downsides are annoying to deal with though, as mentioned in this thread, having to rebase a stack is annoying. I will definitely check out the branchless workflow!
What I find is really useful about this is it forces you to think about what you’re going to do before you do it.
Ironically, I’ve found that better tools for managing stacked PRs cause this to be much less true. When it’s easy to shuffle changes around between dependent branches, you can figure out the branch (and thus PR) structure after the fact without painting yourself into a corner. But without good tool support, it’s a massive headache to change your mind later.
Yeah I’ve not tried any tooling (yet) specific to this workflow is that’s maybe why I’ve found it nice for thinking ahead.
I’m going to try some with the team in the coming weeks though and see how we work!
In two previous jobs were we used Gerrit with mandatory review, i ended up stacking PRs quite tall. It was a rebase nightmare, but the review was made easier because of it.
One pitfall however is that once you have many PRs depending on a single cornerstone PR, it is discouraging for reviewers to fail/disallow that cornerstone PR
I used to do this but it would almost always get messy. If changes are necessary after code review on a PR early in the chain (and you should assume there will be), you need to rebase or merge the earlier one into any PRs that depend on it. It’s often not possible to do this cleanly.
Nope. Hard disagree. You need to rebase. Only rebase, no merge, never merge in this situation. It just fucks up the tree and make hard to figure out what should go where. I only ever merge in one direction, and rebase in the other.
I actually 100% agree, and would only ever rebase in this situation. I only added “or merge” in anticipation of some people who get weird about rebasing anything ever. :P
git-branchlesshandles this well (author here). You just check out the old commit and amend it and rungit restack. This updates all descendant commits and branches. It won’t start merge conflict resolution unless you pass--merge, so you can decide to resolve conflicts either now or later. You can also usegit-branchless’s amend tool to do both in one step.You literally can’t do this with vanilla git or you’ll go crazy. Ben was underselling how hard it is in his post.
I currently use git-branchless thanks to @arxanas hard work. The only thing it’s missing is the ability to sync a branch of pull requests up to Bit Bucket but I’m not expect any tool to get that anytime soon ;(
Interesting, I haven’t noticed this in my experience. But I definitely see the risk with stacking commits that fundamental changes in the base commits would invalidate assumptions used to build later ones
Not sure I follow, but if the early commit has already been merged, and then you realize you have to make a fix before the next one goes in, then those just rebase on the fix – why is that a problem? What is the alternative?
The problem is with thrashing between several PRs and keeping track of which ones need updating and in what order (i.e. if you have a dep chain of 3+ PRs). The other problem is dealing with conflicts, which is just annoying. In my experience you’re just trading one kind of mental overhead for another.
The alternative is to just work on something else while the base PR is under review. IMO this is a social/collaboration problem, not a technical one. :)
Gotcha.
Completely agree.
I would say the solution is small batch sizes and a fast enough PR turnaround time. If you have those things, you can keep working on the next dependent PRs, and even if the reviewer requests larger changes than you anticipated, the rebasing and conflict resolution work should never be unmanageable.
Said another way, batch sizes are too big, or reviews happen too slowly, if the ripple effects of review scare you to the point of pausing work.
You cannot rebase a stack of multiple branches in git with one command. Try it… crazy shit happens.
Well yeah, you’d need to rebase one, merge it, then the next, etc. My comment above about the social aspect is relevant here: everything will be fine if your PRs are small and getting reviewed quickly. If that’s not happening, you’re hosed anyway. This isn’t a tech problem, and you don’t need these other services on top of git to solve it.
I’m not sure I understand your comment. I think we might be talking about different things. If your version control system doesn’t support you continuing to work by building up changes on top of changes already being reviewed you will not create small, granular code reviews. It’s not a social problem that I’m talking about, it’s very much a tooling problem?
We might be. Example to clarify:
My claim is that this works perfectly fine in vanilla git, and rebasing is quite easy, so long as we have small PRs moving along quickly. I guess my question to you is: If you disagree with this claim, what is the situation that breaks it and why do I not encounter it? My guess is that the answer would have to be differences in team size or culture, and not tooling.
“ A rebases Y and Z on X.”
This is what I’m disagreeing with. This doesn’t work well and requires a lot of manual work.
If you rebase Z on X you bring Y’s commits but not the Y branch pointer. If you rebase Y on X you bring neither the Z commits nor the Z branch pointer. You can force move the branch pointer but this doesn’t scale to a lot of stacked branches, since you’re now doing work PER branch, is generally messy and error prone, and is a pain in the neck. People are often surprised by this behavior but if you go and play with it you’ll quickly see what I mean.
To make stacked branches/PRs work, your vcs must make amending an earlier commit a single command, not one or two commands per descendant branch.
I never have these problem and I do what I decribed almost every day, though tbf I generally am only “1 deep” because the “X” will get merged within a day. But this is what I was saying originally: if your “stack” is so deep you have culture problem. Imo, at that point you change the workflow so you have smaller PRs and a shallow queue.
Again, this is never a problem in practice for me, because the pain I’ll feel when I later rebase Z only depends on how far the work has drifted / how many conflicts I have to deal with.
I just played with it in a sample repo and I still don’t get it.
I don’t follow the connection here. In the workflow I use, unless I am cleaning my own history, I am only ever rebasing and fixing conflicts – not amending an earlier commit.
Your vcs shouldn’t dictate how small your changes can reasonably be or how much time you can take for code review. I sometimes write 5 changes and publish 5 code reviews that build on each other in a single day. Maybe you don’t do that, but it’s not reasonable to say “just get code reviews faster.” git fails fast, small work that builds on earlier work. If that isn’t your workflow (maybe your changes all take 1+ days to do) then that’s cool but it just means you do different work.
And code review can’t really take 1 day for everybody, even if the changes are published slowly. Sometimes code is controversial or hard and needs to have multiple iterations.
The main use case for “cleaning history” is that you get code review feedback and need to amend earlier commits. It’s a pretty straightforward use case.
If git fails for you, fair enough. I don’t claim that there is no situation where vanilla git is not enough, only that I have never encountered it using git regularly for a long time.
I still don’t understand your original comment. Is your complaint just that a reviewer asks for a change on a non-head commit in your PR, and in vanilla git you have to
rebase -ito do it, so therefore you need another tool to make that easier?And that’s totally fair! I’m glad git serves you well. But it sounds like you don’t use stacked PRs, or as you said, your stack is never really more than one branch deep.
yep!
rebase -icannot handle a stack of branches. It only works on a single branch. If you are rebasing a stack of branches you will not end with the intermediate branch pointers pointing at your rebased code. So therefore I need another tool to make that easier.If I have these commits/branches:
If
branch Ais in code review and I need to amendCommit 2to make a requested change, there is no way to do this in vanilla git without manually rebasing every stacked branch separately after the amend operation. With other version control systems like Mercurial, no matter how many stacked commits I have the amend is exactly either one or two commands. It scales with the number of stacked commits, or in other words an amend is always O(1) operations, unlike git which is O(# branches). Let me know if that still doesn’t make sense… this comments section is a hard place to explain complicated things.Ah, ok. Your last example clears it up for me. Thanks for explaining. Good to know about if I find myself in that situation in the future.
We also decided to split up feature PRs at work into smaller reviewable units. This is often a bit of a pain when changes needed to be made to the lowest PR in the DAG, since then you need to rebase all the dependent PRs. This will get especially tricky if PRs are squashed on merge. But, having small PRs to review is still a good thing :)
Myself and several co-workers use Graphite. I don’t want to speak for them, but I am a huge fan and would be hard pressed to work in a git repository without such a tool. Not only have my PRs become easier to review, but I’ve felt more confident doing more complex operations in git more easily.
From Graphite’s landing page:
Immediately followed by a “join the waitlist” button. How could it be open-source and have a waitlist at the same time?
If I understand correctly, the command-line tool
gtis open-source, while the review web service is not.gtis an alternativegit-branchless, while the review service is an alternative to GitHub’s UI.Correct. I essentially never end up using the Graphite Web UI. Its the command line tool
gtthat I find incredibly useful and would hate to give up. The web UI is good to be sure, but I don’t find it better enough than the GitHub UI to actually reach for it.I go to the Graphite web UI for one feature: the “merge all” button that automatically deals with GitHub’s braindead approach to reparenting child branches when a parent is merged.
Critical feature: for a reviewer to see the size of a PR at a glance.
If the only thing you know about a review is that you got an email saying that you have a PR pending, you have no way to tell the size except looking at the PR, which is already the biggest hurdle.
That’s why I don’t bother with small PRs at work; they’re easier to review, but they have no way to signal that they’re easier to review, so they have a disadvantage competing for reviewer time.
I use the highly sophisticated solution of putting
[easy]in the PR title ;). This works well when most of the commits in the stack are small things like refactoring, and only one or two are substantial.This is a great point. A couple tools (Gerrit is one of them) display t-shirt sizes of PRs, which I like. (Not sure how good the heuristic is that they use, but I find it easier to judge at a glance between “Small” and “Extra Large” than by looking at diff line count)
What about if someone creates a PR notification service that hits your inbox with all the PRs grouped? Can view in a ‘single PR view’ and also presented as a DAG.
Email is a terrible notification system. I already receive literal thousands of messages per day.
Sounds like the problem isn’t the notification system but that it is overloaded. Email gives you the tools to handle this like filters and subaddressing.
When I received many hundreds of emails a day (admittedly not thousands) my best rule was to prioritize anything that mentioned my actual email, so removing mailing lists from the priority triage. That and unsubscribing from junk was sufficient for dealing with a lot of info on a daily basis. Of course you can always do something more explicit if necessary.
I have a filter which marks emails read when I’m not in the To or CC. This lets me subscribe to high-volume mailing lists without having to sift through everything.
Indeed. I don’t find emails in my inbox, I data mine with gmail search. But seriously, gmail should get good enough with AI to detect and bump up emails it knows you want to read even better than now. I see my gmail will put emails I sent in to the top of my inbox after a few days that need attention to follow up. I indicated I would do just that in the email I had originally sent. I’m terrible at following up. Value add. Conceivable it should know a PR is really important, if you’re obviously using lobste.rs and whatnot. Definitely prefer if all that power wasn’t in Googles hands and some other email service could do that.
Oh I’m not even referring to gmail specifically. At work, where I’m forced to use Outlook, I receive thousands of messages a day where I need to set up almost 100 rules to make email manageable.
I am a big fan of git-branchless. I don’t use it myself as I am quite well versed in git, but it has a lot of optimization that I like and / or wish that I had the time to build myself. Strong recommend from me.
The git-stack repository also includes a comparison of various “stacking” tools.
git-stack should probably link here instead https://github.com/gitext-rs/git-stack
Thanks! I missed that it’d moved (or maybe I linked to a fork?)
This is really exciting to me. I definitely love the mercurial workflow with a dag of commits that individually get reviewed and submitted. Generally the goal is not to have a deep dag when possible, but overall, it is a much nicer flow than adhoc GitHub branches and PRs.
I wonder how well or poorly this would fit with some open source work I do. I assume it wouldn’t work to be the only person using git-branchless or similar tools. From a first look, it seems to match the workflow I find best, but not at all sure about integration.
I really like the mercurial workflow too, and I’ve been chasing a similar experience in git.
git-branchlessis the closest thing I’ve found, but if you need to use branches (e.g. to push PRs to Github), it can be a bit clunky. With Gerrit, git-branchless gets you really close to the mercurial flowThis approach single-handedly leveled up my career when I first started out, it was game changing! What I find is really useful about this is it forces you to think about what you’re going to do before you do it. And that skill becomes incredibly important once you start spending more time planning tasks and less time completing tasks. The downsides are annoying to deal with though, as mentioned in this thread, having to rebase a stack is annoying. I will definitely check out the branchless workflow!
Ironically, I’ve found that better tools for managing stacked PRs cause this to be much less true. When it’s easy to shuffle changes around between dependent branches, you can figure out the branch (and thus PR) structure after the fact without painting yourself into a corner. But without good tool support, it’s a massive headache to change your mind later.
Yeah I’ve not tried any tooling (yet) specific to this workflow is that’s maybe why I’ve found it nice for thinking ahead. I’m going to try some with the team in the coming weeks though and see how we work!
In two previous jobs were we used Gerrit with mandatory review, i ended up stacking PRs quite tall. It was a rebase nightmare, but the review was made easier because of it.
One pitfall however is that once you have many PRs depending on a single cornerstone PR, it is discouraging for reviewers to fail/disallow that cornerstone PR