In Praise of Stacked PRs
benjamincongdon.me
benjamincongdon.me
If you "stack" your changes across multiple inter-dependent branches it looks like "git rebase" is going to learn how to update related branches using a new "update-ref" command (alongside "squash", "fixup", "exec", etc) that gets activated automatically through a "git rebase --update-refs" command-line flag and config option.
(This is from Derrick, not me)
""" This is a feature I've wanted for quite a while. When working on the sparse index topic, I created a long RFC that actually broke into three topics for full review upstream. These topics were sequential, so any feedback on an earlier one required updates to the later ones. I would work on the full feature and use interactive rebase to update the full list of commits. However, I would need to update the branches pointing to those sub-topics.
This series adds a new --update-refs option to 'git rebase' (along with a rebase.updateRefs config option) that adds 'git update-ref' commands into the TODO list. This is powered by the commit decoration machinery.
"""
This is under active development (I don't believe the topic has been merged yet) so it's still open for feedback and refinement on the git development mailing list. Thanks Derrick!
[1] https://public-inbox.org/git/pull.1247.git.1654263472.gitgit...
That obviously doesn't work if you modify or drop any of the commits, so this option is very welcome!
Keep in mind, though, that humans are not perfect and some choices are unwise.
The work of breaking up a big, inspired chunk of work into small pieces helps you learn more about it, and the perspective can reveal improvements that weren't obvious in the initial effort. You might notice those yourself, or reviewers may. The final outcome will end up overall better for it, so spending that time is worthwhile.
There's a tradeoff to be made. Have a feature sooner or later. Review now quickly and more carefully later, or review carefully now. Part of what development teams do, is risk assessment.
Put a feature flag on it, do a demo of the branch. If it looks good, do a quick once-over to see if it's interactive with any limited resources, merge it in, make a ticket for a re-review later.
I haven’t ever worked at big corp so maybe this kind of thinking is actually valuable there. But in most startups in my experience this mindset is wrong. You literally won’t have a job tomorrow (because your company will fold) if you don’t ship value-generating product yesterday. But you’re going to worry about how inconvenient it will be for some other developer to review your PR?
I wouldn't want to work at a startup that doesn't value that in their engineering culture, at least not again. More mature teams (in terms of the staff, not the business) I've been on get this and ship quickly.
It's not about humongous reviews being "inconvenient". A drop of thousands of lines of new code takes a long time to review thoroughly, whether the company is at risk of folding tomorrow or a stodgy enterprise with decades ahead of it. If you have to constrain review time - or ditch reviews altogether, why not? - you can, but there are consequences regardless.
I haven't worked in early startups, but I haven't only worked at large corporations either. I hope that most of us, most of the time, aren't less than a business day away from unemployment, and so there's usually time for code reviews. (If not, there's a lot of useless material about it!)
Without sarcasm, and potentially getting off-topic, I would like to hear stories about how a startup survived impending doom with heroic, fast shipping of product that set aside a lot of time-consuming processes. What were the most crucial steps to keep? How was the technical debt repaid?
All of them have something in common: startup needs money so you demo to potential customers or investors. Unlike in a stable corporate environment where deadlines can have flex, you really don't want to cancel or postpone a meeting to sell to a client - so those demos dates are set in stone, and if things aren't ready you will need to pull some heroics.
One memorable one was when three of us spent the hours leading up to a demo disabling automation and deploying to production by hand. What that meant was disabling a bunch of tests and code that checks those tests succeed in order to get the code out the door. We spent the next week cleaning that up. Sometimes you need to test in production.
Another was actually a demo for the same client that set up that meeting. This time we had heard from the initial introductions they had a specific, very nasty problem and we could ostensibly solve it (that was true, we could, but the product at that point had no facility to do it). So I spent I think three days hacking together a solution that would solve the problem for the contrived demos we would show. That solution had atrocious performance characteristics so we wound up spending a few story points over the next two sprints to optimize and refactor the guts.
Yet another demo was an integration. We had scoped out the general feature (which was magical when it worked) but it required a large amount of effort to coordinate with the original software vendors to get the data we would need. But those vendors have a tool with a free trial install which had a bunch of the data in XML, so I spent a day reverse engineering the schema and a second day parsing into our internal data model which we demoed on the third day to show how our product could solve that particular problem. We never wound up getting data from the first party company, and the parser got rewritten in a different language, and the backend that does the stuff is planned to be replaced in the next few sprints once it gets priority.
So TL;DR you have a demo scheduled, you hack together a thing that can be presented, then spend your time reimplementing or refactoring the demo code into something maintainable.
It helps to have a stack you can really quickly iterate on. And management that understands demos aren't suitable for production.
When I've hacked together stuff like that I'm often making it up on the fly, I rarely have a plan in mind that lines up with how I'd want to present it for code review later. Hell, a lot of times some of the later PRs would simply remove the entirety of a failed earlier attempt!
> Well, I'm proceeding from the assumption that thoughtful review, which takes time, is desirable
Ofc it's desirable. Nobody is arguing this.
> If the situation is really that dire, then it's even more important that you ship product that works.
That is simply incorrect. Broken features (in innumerable shades) are debuted all the time to secure future investment in various ways which are not just financial. Sometimes it's community confidence, sometimes it's leadership clout, etc. Betas are a thing. This fear-mongering about 2500 lines of code is pearl clutching that hobbles large organizations that either fail to deliver or fail to deliver stability, regardless of their processes that drags on development for 4x or 10x, which they have misplaced confidence in.
> I hope that most of us, most of the time, aren't less than a business day away from unemployment, and so there's usually time for code reviews. (If not, there's a lot of useless material about it!)
There is lots of useless material about it. The number one rule is "don't interrupt the flow of money" but that rule doesn't extend to every project forever. Large organizations get less and less efficient over time (see rule 1) because perfect is the enemy of good. It's a good tradeoff, when you have an organization that can't keep up with how much money they are making to start piling on additional process because you don't know where your weaknesses are so anything that goes wrong gets an additional check. Most orgs are not in this boat.
If you don't look like you're making your money's worth, even if you can explain how some side-process adds value, someone starts looking for your replacement and eventually they will find one and you're gone. That's true, regardless if you work at some mom and pop shop or Amazon. You're always one day away from unemployment, even if you don't know what day the chain of events started. So code reviews that block features can hurt you and have put a lot of people out of work, I can attest. Ofc we're talking about more and more extreme situations, but the idea is the same. Code reviews are a scale and that scale has a tradeoff.
The gut-check is when a company has a merger/acquisition deal. The team doesn't have a lot of time to vet the other company's codebase. You do your best to have your info meetings, try to run parts of the code with data, make your recommendations, and the escrow eventually closes. 2500? How do you think you'll do when looking at millions of lines? At some point, you will be forced into black box testing and realize that this is what matters foremost. Look at the interface, look at the expectations, look at the data. After that, the rest is gravy that can be addressed later.
If someone wanted a PR review for 2500 lines, I would tell them no. I absolutely expect a developer to think twice when trying to make a change that large in one go.
When your corporation’s billion lines of code are the engine that helps generate $30,000 a minute, you don’t just say “meh, looks good to me.”
Furthermore, this is multiplied on my team, which is the framework team. A mistake in our auth middleware, http client, etc, could easily turn into a mistake on 50+ other teams.
Once you have significant runway and can hire, you need to be able to start delivering faster and faster. One person writing 2500 lines of code in bursts once or twice a week isn't going to scale. You need a team, and you need to be measuring total team throughput.
Need a new service prototype? A 2500-line PR is appropriate. But don't be surprised if it needs some big revisions - that's the point of a prototype anyway.
Need a feature? One person periodically "when inspiration and time line up" dropping 2500 lines (this is a LOT of lines in most languages used these days, even in newer, more-concise Java versions) on top of what 10 other people may be working on is not going to help everyone else move quickly at all.
If you're the person who's so busy you rarely have time to code, you need to figure out how to turn your inspirations into ideas others can execute and be the code-level experts on. A team of 10 "1x" developers is still more productive than a single "10x" developer who's in meetings figuring out the company's plan half the day anyway.
Either way you should be looking for a new job, because what you're describing is quite unsustainable in a team of more than one person working on the same thing.
What's the alternative anyway? If you don't want me writing 2500 lines of code in one area, would you rather I write 10 250 line PRs in 10 different parts of the codebase instead? Is that supposed to be easier to review?
Or is the rule just "don't write a lot of code in any short period of time"?
Depending on the team this may be a completely reasonable policy.
1. Deprecate old feature + add opt-in support for replacement 2. Make replacement default with opt-out for old pattern 3. Completely remove old feature and the opt-out functionality
I can write the entire stack of diffs upfront, have them individually reviewed but still linked, and ensure they're merged in the correct order.
The bottleneck for merging isn't in the review process, but in the deprecation. It wouldn't make sense to land all three of these changes as fast as review/merge would allow; that would skip the deprecation period.
Kicking the completion of the implementation of a change you've started landing to an unspecified future date indicates poor engineering rigor imo. It's just begging for the change to perpetually be half-migrated and never finished.
It totally is, because it doesn't wastefully discard the mental context needed to make the follow on changes. Task switching unnecessarily incurs significant costs.
> You could spend the same time on other changes that would start earning you money tomorrow, instead.
Maybe in some bare-bones startup context that can't afford to think beyond next week, but most organizations aren't like that.
Consider also the stakeholder who gets annoyed whenever the dev team wants to work on something that would take longer than a week to turn around, and limits the things they'll ask for to those estimated at a week. So bigger things can never get done at all, and you'll just be looking for a local maxima instead of having the chance to make more significant changes.
Sometimes it's worth it to prep and clean up as you go. Knowing when it's worth it and when it isn't is one thing that makes some devs more valuable long-term than others.
1. Introduce new test exhibiting bug
2. Introduce bug fix and update the test
If 1 and 2 are reviewed together you have less evidence that the test actually shows the bug being fixed.
We actually have a rule that there must be 1 commit just introducing a test that fails on CI for bugfix PRs.
"rule that there must be 1 commit just introducing a test that fails on CI for bugfix PRs"
EDIT: at least in rebase workflows which is what I'm used to. I guess in merge workflows this works.
1. Merge #2 into #1 then merge the result, but this can obscure your history
2. Don’t make the test fail. Eg expect the incorrect behaviour and leave a comment explaining why it is incorrect and what the correct thing should be.
Unless PRs are merged instantly, I'm always going to be waiting after one PR is opened, before I can work on the next, unless I stack, aren't I?
Is your definition of 'fast enough' instantly? If not, how does this work?
Also, trying to make units of work so that they don’t need to overlap like that can be useful too
See, I disagree, because this is absolutely the place where it helps the most—you can now go away and keep working on the same task without having to context switch to anything else or remember where you were or what you were working on. So you're never in a state where you're blocked and can't work on your ticket, and you don't have anything else to do or have to pick up another ticket and start learning about a whole completely separate problem. (And yes, definitely everything still needs to be written down, it's important to walk away knowing which changes you need to make, and why!)
> it might still be some time before your coworker has available time to participate in such a session, so it's still likely you'll need something else to work on in the mean time.
In teams I've worked on, the expectation is that an engineers highest priority is always unblocking another engineer, so this very rarely happened. Unless they had an interview to go to or some kind of meeting—and in that case, you could always just ping somebody else.
Obviously it's a style of work that relies really heavily on everyone sharing the same timezone and work hours, but it works really well to eliminate time lost to context switching and minimize the amount of time engineers spend blocked waiting on someone else.
Generally we want to reduce any accidentally complexity and the way to achieve that is to reduce the latency of code reaching production. So for example you want to minimise PR/branches in flight ideally to 0. So you should be asking how you can reduce them - eg trunk based development, omitting asynchronous code reviews etc.
Huh? Nothing I said has anything to do with "the cost of integrating changes". I'm saying that it's more efficient for developers to work on one single task at once, instead of trying to "multi-task" among a variety of unrelated changes, and that synchronous code reviews help with this because they minimize the amount of time a developer spends blocked waiting for feedback from the rest of the team. Certainly this also has the benefit that you get fewer merge conflicts and have fewer branches in flight, but I don't know why you're saying that synchronous code reviews are "just externalizing the costs of integration [...] on to other people". That doesn't make any sense.
Another way is that you next work on something different enough that it doesn't need to stack. E.g., reviewing pull requests.
I've seen no solutions, only tradeoffs but I'm curious if anyone has a tried and true way to avoid this traffic jam scenario.
For example you do your first PR, mark it ready for review. While doing it you notice there's some refactoring you could do to some tangentially related code. It's very conceivable that the second refactoring PR could be ready pretty quickly.
Yes there are other situations, but in my experience, the ones I mentioned are the most common.
You can (/will have to) rebase later.
* you're working on the same part of the code
* you aren't working on the same part of the code
The second scenario is common but also trivial here since you can just have parallel branches going, so I'm gonna assume more the first - working on something that's building on top of what you just put up for review.
Let's say the review is done in 2 hours. If you're already done with the followup, IMO you may be erring too far on the side of "small PRs." If you aren't, you just rebase on top of whatever changes had to be made, if any, to the first one, once it lands on the primary branch.
If, on the other hand, the review isn't done for 2 days... then that's a PR turnaround time problem for sure.
But I strongly disagree with the people saying "multiple dependent PRs suggests the work wasn't split up properly" - there's nothing worse than a mega-PR of thousands of lines for the sake of doing a "single feature" all in one shot vs having to possibly pause and rebase periodically after review. It's even more painful when this mega-PR requires fundamental changes that could've been caught earlier, but now will take longer to adjust, and then will stay open for a while and likely result in merge conflicts as a result.
One of the projects I work on is a large, well-known open source project. Reviews that take days are pretty common.
> If you're already done with the followup, IMO you may be erring too far on the side of "small PRs."
There have been times when I created a sequence of three or four separate commits within under half an hour.
This was on projects where test coverage is tricky (think hardware interfaces) and keeping changes small was motivated a lot by better bisectability.
Clearly, software development is a pretty broad field with lots of different experiences.
Yes. Whenever you inspect a proposed solution, you should hopefully find the problem that it solves.
With software it can be harder to notice because you don't have to make room for it. But in essence it's the same deal; it's anything we have paid to create that isn't yet delivering value to the people we serve. Plans, design docs, and any unshipped code.
There are a lot of reasons to avoid inventory, but a big one is that until something is in use, we haven't closed a feedback loop. Software inventory embodies untested hypotheses. E.g., a product manager thinks users will use X. A designer thinks Y will improve an interface for new users. A developer thinks Z will make for cleaner, faster code.
Both large pull requests and stacked pull requests increase inventory. In the case of incorrect hypotheses, they also increase rework. I could believe that for a well-performing team stacked PRs are better than equally-sized single big PRs, in that they could reduce inventory and cycle time. But like you, I think I'd rather just go for frequent, unstacked, small PRs.
[1] e.g. https://kanbanize.com/lean-management/value-waste/7-wastes-o...
Small prs don’t need it of course but complex features benefit from shaking out things earlier. Commit more than 100 lines are really hard to review (lots of anecdotal and empirical research). If you’re not reviewing small commit by small commit, the reviews are easily missing things. A single PR that’s 800 lines adds review time to go commit by commit. If you can merge the non objectionable stuff, the reviewee gets to feel a some of forward progress and fewer merge conflicts (eg someone lands a refactor before some simple change of yours vs your simple change handed before and you made it the person refactoring their problem where it belongs)
If there's a simple refactoring everybody agrees is good whether or not your overall goal ends up making sense, then yes, by all means merge that. But that doesn't require stacking unless your review process is slow. In which case I still think the right solution is to speed up review, not to stack.
For the cases where we don't know the full story, my first thought is that we never know the full story. So there I try to instead find the smallest unit of work that everybody agrees is a step forward.
When that's not possible, where the unit of work still seems pretty large, instead of breaking that up into a bunch of stacked diffs that shouldn't be merged until we really understand something (which to me sounds like a large PR in disguise), I think a better option is a spike, where we intentionally do a quick, throwaway version of some change as a way of learning about the change. Instead of trying to do good code along the way to good understanding, we just go for the understanding. Once we have thrown out that scratch code, we then go back with our new knowledge for a proper PR.
So I'm still not seeing where I would use stacked diffs, except in this case here: https://news.ycombinator.com/item?id=32215346
The premise of stacked diffs seems that we won't learn anything significant from reviewing or deploying code. (If we did learn something valuable, then the things stacked on top could be up for a lot of rework or might be thrown out altogether.) I think that has a lot of bad effects, but one of the biggest for me is that the bigger the inventory of code (whether in one big PR or a stacked set of smaller ones), the more a reviewer will feel obliged to say, "LGTM" and let it go, because they know there's not much point in saying, "Actually, I think this whole thing could be better approached by X."
So like you I'm entirely for small, reviewable lumps that are easily merged. I just think they should be then reviewed quickly, so that stacking or agglomerating is unnecessary.
This feels like a critically important point to your position but there's no actionable advice provided on how to achieve that. FWIW I've found stacked PRs do speedup reviews because it pipelines the work. Pipelining something removes bubbles (in this case time spent waiting on review) from forming.
Simpler parts of the work get eyeballs from more junior engineers who feel more comfortable approving smaller / simpler PRs (& other people feel confident in that). Trickier stuff is left to the smaller pool of people who have the appropriate context / skillset. If you're waiting on landing 1 PR at a time, then you're serializing the review flow which means your total time on the PR is "time spent writing + time spent reviewing 100% of the code". If you pipeline your stacked diff, then potentially you could get ~80% of the code reviewed & landed by the time you finish the more complex pieces. Then you're left with "time spent writing + time spent reviewing 20% of the code". Additionally, by putting up the commits early, you're letting other people fit smaller reviews into their schedule more easily vs "here's a PR with 5000 lines of code changes" which is a monstrosity to review (i.e. quickly runs into mental fatigue issues / quality of the reviews can easily degrade, especially if commit hygiene wasn't practiced).
Have you actually tried stack PRs with a proper review process & good commit hygiene? This is one of those "try it before you knock" it things.
If your point is that in some existing organizations stacked pull requests are better for a specific engineer's experience than doing all the related code in a big blob, I certainly believe you.
Similarly, there are manufacturing shops where reducing inventory in line with Lean approaches doesn't work out of the gate, because there are other organizational problems/constraints that have to be dealt with first. For example, you might need a large buffer of component X at stage Y of a manufacturing process because upstream quality issues mean that a smaller buffer would cause frequent stalls at stage Y. First you have to fix the upstream issue before you can cut stocks there.
So are stacked pull requests the optimal choice for some specific person on some specific occasion? Sure! I'll take your word that's the case for you. What I'm saying is that I think they're an indicator that there is some systemic problem that could be resolved so stacked pull requests and giant pull requests both become unnecessary.
I guess this is not true anymore post covid outbreak? Pretty sure a lot of companies would kill to have inventory of their raw materials right now...
It's true that pandemic supply chain issues have change the level of necessary waste in a lot of supply chains. But that doesn't make inventory good. Often production halts not due to everything being missing, but a shortfall of just one input. A company might mistakenly react by stock up on everything, but that still won't solve the shortfall of the critical component.
So should the just stock up on the critical component? Go get a year's backlog of that? If everybody does that, that will cause a shortfall all on its own, as when everybody did panic buying of toilet paper in 2020. And then when supply chains straighten out, then the stockpile is back to being unnecessary waste. So I don't think there are any simple answers there.
Stacking PRs is like pipelining for CPUs. It's efficient under the hypothesis that there aren't too many invalidations/stalls. The linked tooling `git-branchless` (I'm the author) is aimed at reducing the impact of such an invalidation by significantly improving the conflict resolution workflows.
Depends on the team and the product. My personal approach is to have 2-3 larger things to work on, so while I wait for reviews on one, I can switch and work on the other. This usually means minimum 1-2 weeks of planned work, sometimes even more, without being blocked on reviews. If everything is blocked, then it is time for some code health cleanup, refactoring and fixing those TODOs that are just lingering around, and also nudging the reviewers...
Not to mentioned getting reviews for smaller, atomic changes is just SO much easier. Even on a team where everyone is using a stacked workflow, if anyone submits a larger PR (especially more than a few hundred lines), you can see how the smaller PRs submitted in the same time, often in the same stack, get reviewed much faster.
> Stacking in my personal experience usually leads to merge conflict hell as changes and PR suggestions get merged underneath you.
That's been my experience too.
And beyond the technical issues, the deeper you work on a single issue the more at risk you are of the simple issue of finding design or requirements issues in the base MR that require going a different direction and discarding the whole stack. So even if you can somehow avoid conflict issues, stacks are still dangerous.
You can also change the definition of done - something I try to do when a task gets too large, spin off new sub-tasks, etc. The first feature doesn't quite work right, but the merge differential is usually smaller for tweaks or bug-fixes than it is for major initial features.
You can spend a lot time planning how best to break up your work, and sometimes you may be able to do that successfully, but often times it you are constrained not by git but by your tracking tool (jira, trac), which impose somewhat artificial sizes on code tasks....
And really that's the whole thing stacked prs or commits are trying to solve, minimize the merge differential.
Mostly because stacked PRs are usually not ready for review until the base PR is reviewed, and are probably going to get rebased and require some refactors. I've had cases where the base PR had so many changes that I just started over on a new branch.
I know that GitHub now has draft PRs, but I still think that (unless you really want someone to take a look at the draft while the base PR is not merged), it might be better to not make people waste time looking at code that might suffer heavy changes.
This often means first a code cleanup that doesn't change any functionality yet, but makes the later changes simpler - eg; removing dead code, removing unnecessary abstractions/interfaces/layers, upgrading external packages.
Sometimes I recognize this early, and will specifically make a branch for this. If there's changes from the review I'll rebase my next branch(es) on top.
Often I don't see this will be needed until later (or rather: I'm focused on the change itself and not the PR experience) and so I'll interactive rebase to put all the refactor commits first, and make a PR for those.
Both cases mean I already have a branch built off an unmerged PR. Sometimes even before the PR is published, actually. I don't see any particular problem with this. My development style and speed is independent of the speed PRs get merged; the ability to rebase makes this a total non-issue.
Like the Linux kernel for example. You don't get to push half finished work with projects like that. Actually, you don't get to push at all. There is only pull. Some branches exist for many months or even years and there could be entire teams collaborating on them. The one absolute certainty you have on such branches is that there is an absolutely insane amount of upstream changes all the time. The only way to stay on top of that is to merge those changes often.
You can think of the Linux kernel git as a decentralized network of stacked branches that have a few central people pulling changes from branches they have reviewed into their own branches with the one that Linus Torvalds maintains as the ultimate branch on which releases get tagged. The vast majority of changes land to his branch via multiple layers of other people, each with their own branches and each adding their own reviews. Effectively all changes Linus Torvalds integrates are stacked. And he doesn't integrate them unless they are stacked properly with nice clean histories.
You could say, git was explicitly designed to do stacked branches at scale. So, it's kind of ironic that people are re-discovering this as a thing. It always was intended to be used like this. It's been used like that since the very beginning.
Compare with a construction site: there are obvious uses for tools and there are less obvious uses and I haven't been on a single one where tools are only used specifically in one way for the entirety of the project. They’re used dynamically by different people with different experiences in order to complete the project. Nobody forces other workers to use a specific grip when holding their hammer…
For example, Stacking PRs keeps the author unblocked. Authors don’t need to wait on a particular change to be merged before starting to build something on top of those changes.
This will fundamentally be up to the author's git skill whether or not they are presenting the PRs to reviewers / mergers as stacked. If they're skilled git users there's little to no cost presenting them one at a time and keeping the not-yet-PRd branches fresh. If they're not skilled git users, they have no hope of managing multiple PRs effectively only be virtue of presenting them all at once.
Or: Since stacking PRs allows you to create a DAG of dependent changes, this natually allows you to manage code changes that need to be submitted in a particular order.
Assuming this is in a single repository, a fast-forward-only commit policy alone ensures this.
Or: stacked PRs use branches, and can have multiple commits in a single atomic change; stacked commits use a single commit as the unit of atomic change.
Stacked PRs usually use branches, but stacked commits also still use at least one branch and could have more. In both the commit is the unit of atomic change, because that's what a commit is.
I'll also add I find the entire language around "branchless" workflows in git confusing, not just from this author. There is no such thing; considering one special branch as "not a branch" or your local and remote and someone else's remote as the "same branch" just because they have the same name is a holdover from older/other VC tools. We don't do any new git users a pedagogical or practical favor by clinging to that view.
Another term you could use is "anonymous branching". This is not technically accurate in the above workflows, but it captures the essence pretty well in Git, more so than "branchless".
In particular, since this is the one I see usually called out as a benefit of branchless:
Stock Git does not have good ways of rebasing a sequence of branches.
A sequence of branches can be rebased by rebasing (or otherwise rewriting) the longest one (the only one you'll need locally) then pushing the individual commits in the current branch to the remote under any relevant branch names. This doesn't take zero time, but with good git UIs it will take less time than remembering `git move`, and it's not especially hard to do with the stock CLI either.
Sure, I'm only responding to what you were saying about "There is no such thing; considering one special branch as 'not a branch'". There is such a thing in that there is no branch involved in the detached HEAD state. It's not some kind of Git misunderstanding. I think you might be referring to trunk-based development and always building off of the remote main branch instead of having your own local copy, which is unrelated to being "branchless", for the reasons you stated.
For many people (particularly those on Github!), a branchless workflow won't help, so you're free to not use it. In my opinion, it's a workflow that is better compatible than stock Git with code review tools like Gerrit and Phabricator.
I personally argue that anonymous branching is useful even in some branch-based workflows. Mainly, if Git branches are so lightweight to use, why do we also use the command `git stash`, instead of just always creating a new branch for our temporary work? One benefit of anonymous branching is that it consolidates these workflows in a convenient way. Some people don't stash changes or feel that branching in those cases is heavyweight, so anonymous branching doesn't help them at all.
> then pushing the individual commits in the current branch to the remote under any relevant branch names
If I'm understanding correctly, every time you rebase the longest branch, for each commit in the branch, you would manually run e.g. `git push <commit> origin:my-branch-name`? That seems like it would take a lot of time to me. Is the tacit assumption here that you don't have a lot of commits in your branch, so this doesn't take a lot of time?
But at the very least there's still an existing remote branch, which is the ultimate merge target, for example - perhaps even multiple. Since we're talking about coordination, there's also the actual state at the remote vs. my view of the remote vs. my teammates' views of the remote. But we don't think about this too much, because we can synchronize easily as long as the remote branch has a name. Taking the name off the branch makes it more difficult to do anything with someone else's work other than merge it, e.g. pass a changeset back and forth or hand over a half-complete task.
> why do we also use the command `git stash`, instead of just always creating a new branch for our temporary work?
I have no idea actually, because I don't. I haven't run git stash manually since I learned about autostash, and even before that it wasn't for temporary work but for changes I decided I wanted somewhere else only after writing them. Temporary work mostly does get a branch (usually as a new commit on top of my current local branch).
> If I'm understanding correctly, every time you rebase the longest branch, for each commit in the branch, you would manually run e.g. `git push <commit> origin:my-branch-name`?
Close, except I'd scroll down the list of commits and type `P o RET TAB branch-name` (or something to that effect, it'd be slightly different if I have several remotes). This would take perhaps ~0.5 seconds per branch and not require me to context switch to a terminal.
It does take longer for people using less powerful clients, but virtually all do provide some way to do it even it means a few right-clicks on a menu and clicking some "OK" buttons, and there's value in everyone using analogous operations. And the people using those clients usually can't reliably recover from a large class of "git broke" mistakes, assistance from git-branchless or not, so handing them a CLI and a prayer is out of the question. And even the "long version" is pretty negligible (like, maybe 10 seconds per branch?) compared to everything else you should do to make your changeset approachable for review.
> And even the "long version" is pretty negligible (like, maybe 10 seconds per branch?)
I notice that mainly the differences in opinion with regards to workflow is disagreement about "how long is too long" for various operations :)
10 seconds would certainly be too long for me. It is obviously not too long for them, or they'd learn some keyboard shortcuts to get the easy 5 out of the way to begin with. But regardless, it's all pretty much dwarfed by things that need actual brainpower like re-reading my commit messages for grammatical errors / broken links / etc.
(I’m not familiar with Phabricator, only Gerrit and GH/GL, as well as some more manual Workflows.)
As far as I know, Phabricator does not let you view or comment on individual commits in a code review (which I use at work). Let me know if you know differently and I might switch to doing that.
Can you advise me on running CI on each commit in the PR in GitHub? As far as I can see, it's technically possible in that you can run arbitrary code as part of your CI, but there's no convenient way to do it. (In particular, I would want to be able to view the CI runs for each individual commit, i.e. render the little checkmark/x next to each commit in GitHub, which seems like it would require a lot of integration via the API.)
One thing that often happens in a stack is the earlier commits are regularly merged into the main branch while the later commits await review. This helps to keep things in sync in a trunk-based development workflow. Is there a way to split out and merge only the earliest (reviewed and accepted) commits, and leave the later commits pending review, in a tool like GitHub?
Overall, I would like to be able to use GitHub PRs for big stacks, but there seems to be a lot of friction in doing so, to the point where it's more convenient to adopt an alternate code review tool (like Graphite) or PR management tool.
Isn't there a "commits" tab when viewing a revision? Also Phabricator is no longer maintained for the last year.
> Can you advise me on running CI on each commit in the PR in GitHub? As far as I can see, it's technically possible in that you can run arbitrary code as part of your CI, but there's no convenient way to do it. (In particular, I would want to be able to view the CI runs for each individual commit, i.e. render the little checkmark/x next to each commit in GitHub, which seems like it would require a lot of integration via the API.)
You'd use the https://docs.github.com/en/rest/commits/statuses API to publish the little checkmark. More info: https://docs.github.com/en/pull-requests/collaborating-with-....
However, before you do that, consider: this one I think is a slight impedance mismatch between how users conceptually think about using GH and how GH actions are designed to be triggered. The most simple thing to do if you want every commit checked and tested is just make a GH action that runs your checks on push, which gets run on every commit and updates the status. If you already have a push action, just remove the "main" branch restriction. The PR action is designed to do PR-scoped things. You can, if you really only want to check commits once they are opened for PR, iterate over the commit list and manually trigger workflows to test and check each one (you can even just manually trigger your existing push action if you add the manual trigger option and specify the commit sha yourself and it will run, I think). But, is the list of commits that get pushed to the repo and the list of commits you want to eventually test for inclusion into main really that different, at the end of the day?
More often than not I've seen such limits used to "reign in" "misbehaving" engineers or because, as I've suggested, people sometimes don't know how to use a tool to its full capacity. Sometimes it's purely people wanting to force their preference on others. I've had to walk senior engineers through (or just manually clean up when they clearly have no idea how git works) branches that people royally mess up because they merged something in 8 different ways because some other person told them rewriting history is evil or some previous organization taught them to only push merge commits to PRs or protected all branches or something...
There can both be organizations that need to enforce policy to make their business successful and people that don't really understand the full gamut of what's possible with a tool and restrict workflows to stuff they understand. I don't think I equivocated the two. I simply expressed frustration from all the times I've anecdotally encountered people to aren't fluent with git making decisions for others who are fluent with git on how git should be applied.
On multiple teams I've worked on, this has been explicitly discouraged because reviewers often want to see the changeset in as much context as is available, including future changesets.
Most all CI tools support per-commit checks with simple configuration/scripting of their execution DSL/API. In GitHub, push actions already works this way. You can do the same thing on any branch after a PR is opened: on synchronize you can iterate over all the updated refs, run checks, and publish results to the commit status API https://docs.github.com/en/rest/commits/statuses, and https://docs.github.com/en/pull-requests/collaborating-with-....
Can you explain how these are not just facts? We’re not using stacked branches because it has enough downsides, but these are indubitably upsides of stacked branches.
Formatters do nothing for code quality. Linters can help quality but it's more of a last line of defense for common errors, not a signal that your code is 'good'.
And git is a very flexible tool with a lot of generic functions. Pull requests themselves are a prescriptive style of using those generic functions. A "Branchless" workflow is not the author failing to understand git. It's using those functions differently.
And I fail to see how it's worse that workflow constraints may be placed on a very generic tool like git to work better with more specialized tooling in other places.
"I can put any text in a commit message, so why do I have to start it with this specific text?"
"Because that's how our organization tracks work, and the ability to track historic work in this way is more valuable to us than you being able to write whatever you want at the start of a commit message"
PR1 -> PR2 -> PR3 -> PR4
The reviewer reviews and suggests a change in PR1, this change causes a merge conflict in PR2 and therefore in PR3 and PR4 as well. And then you have to go in manually resolve the exact same merge conflict all the way through you PR stack in each of the PRs. This gets annoying and hard to work with.
Does anyone have a better way of dealing with this pattern of merge conflict? I've tried using git rerere which in theory sounds helpful but doesn't seem to do anything when I strike this issue.
Using a rebase workflow with stacked/cascaded PRs is an anti-pattern that trips up people used to other git workflows where the history depth is effectively only one level deep instead of arbitrarily deep as in stacked/cascaded PRs.
If you've built up some trust with your team, they shouldn't have a problem with it.
To rebase your commit stack on top of the main branch, use `git sync` instead of `git merge`. Merge commits often make it so that you have to resolve conflicts multiple times.
For example don’t mix formatting changes and logic changes in a single commit, or don’t fix two separate things and lump them together in the same commit.
It takes a bit of practice to figure out what constitutes a logical thing - you don’t want them too small otherwise it results in lot of noise, and you don’t want them too large otherwise it defeats the main purpose of using them.
A good rule of thumb is to avoid words like ‘and’ in your commit subject line e.g. if your commit message subject contains wording like ‘Do this thing and that thing’, you’d probably be better off having two commits ‘Do this thing’ + ‘Do that thing’.
This doesn’t fix the problem entirely, but I’ve found that using atomic commits greatly reduces merge conflicts when rebasing, and when there are conflicts they are usually easier to manage and don’t result in as many follow on merge conflicts.
0: https://www.aleksandrhovhannisyan.com/blog/atomic-git-commit...
Its extremely close to magic. You make all the requested changes on the PR4 and type hg absorb and it'll figure out which commit in your stack each change belongs to.
I've used this to work on lots of stacks 20 commits deep in mercurial.
I understand some of the "no, merges are superior because they retain all history" proponents (not claiming you are one, just aiming to head off that flood), but all those dead ends and refactors along the way are just those - dead ends and refactors. They ruin the ability to bisect, for instance, because they are rarely all functional. Intentional commits, like a stacked workflow supports in a tool like Gerrit, lets you have a wonderfully understandable history that works at every point.
An alternative solve is to work in a way that allows PRs to be merged more quickly, ie pairing, mobbing, or prioritizing getting reviews done asap.
In my experience with this sort of process you spend quite a lot of time managing your different branches, especially once you start getting feedback and requests for changes. Then keeping everything in sync. Git helps make this quicker but it’s still effort & cognitive load.
Getting folks to quickly review PRs is difficult as doing so breaks the flow state of other people and results in constant thrashing and context switching between the work they are doing are reviewing PRs.
Even if the team is committed to quickly reviewing PRs, you are always going to have times when a bunch of people are in a meeting, while a couple are at lunch and a couple are knee deep in another issue and no one can look at your PRs for hours and you need to keep yourself unblocked.
I previously worked somewhere that used Phabricator. Its “stacked diffs” worked great. I’d use it all the time when working on complex, multipart changes.
Is that the argument intended to make?
So that's why you get to: stacked PRs and whether or not they're good.
Rewind to the place where you decided to use PR process in the first place and consider if that was a wise choice.
I never need to stack PRs cuz I can work on the next one while the first one is under review and rebase once it's merged. I can see if the review phase is long at your company that this wouldn't work. But I prefer it if possible.
One thing I hate about stacked PR delivery is ppl go dark for a month building this whole new world and if you have architecture concerns in the root PR they will resist them because it means rebasing and changing all the downstream PRs as well. Bad incentives all around.
> ppl go dark for a month building this whole new world
I think this isn't specific to stacking PRs. You can put 100 commits in a PR and submit it at once, or you can make 100 PRs and submit them all at once, and have the same problems either way. Ideally, you put up your first few commits or PRs early so that they can get reviewed.
Maybe just a matter of developer discipline, but in my experience people tend to create large stacks of 3+ PRs that then take a while to resolve. Yeah sure, without these tools the code would also exist somewhere, but at least you don't have your pull request list full with PRs that aren't yet ready because the upstream PRs haven't been approved yet. You anyways have to (or should) review things stricly serially, so the additional PRs existing are not super useful either. Maybe as context for the reviewer to see the future work.
Also for some reason these tools seem to encourage people to put unrelated changes in the stack that could be based directly on master. Probably because they are anyways working on their stack and integrating some unrelated fix they just did into the current stack is easier than rebasing/changing your current context. But that's just a minor pet peeve.
That said, I would still like a tool that lets me manage stacked branches just for myself. Not exposing them as Github PRs or so, but to organize my work into different branches. Executing a chain of rebases of branch N -> (N-1) can get quite annoying manually. Probably some arcane git magic to (interactively) rebase branch N->(N-1)->...1 in one command exists already.
There is not really a command for that yet, short of adding a bunch of `exec` steps to your interactive rebase manually. See https://news.ycombinator.com/item?id=32217204 for an upcoming command.
You might enjoy using https://github.com/gitext-rs/git-stack, which specifically tries to let you manage stacked branches locally while not exposing tons of PRs to your coworkers.
git-branchless itself also lets you manage stacked branches in various ways. For example, you can do `git checkout <branch>`, `git commit --amend`, and then `git restack` to rebase all the descendant branches sensibly. You can use it on the local side of things only and then use Github PRs as normal.
This tool is really nice and related to these types of workflows.
This git patch stack tool is really nice. You don’t really use branches at all with it. You just build up a stack of atomic commits. At any time, you can request review of a patch, and it will cherry-pick the commit onto a new branch off of master and create a PR for it.
What you describe appears to reduce variable costs (time spent reviewing) while increasing fixed costs (context-switching). That may increase code reviewer usage, but that doesn't necessarily increase system throughput or reduce WIP.
My naive sense is that the knowledge should exist to organize arbitrary code changes in to “good”, “readable”, segments.
I mean hell, that’s a whole business right there
And then there’s the problem of large codebases. Without a really good cache strategy for builds all of the branch switching and merging will cause a lot of rebuilds. For some languages and large projects that can be 20+ minutes for a fresh build on each switch. That’s enough time to get distracted by emails or hacker news.
I’m a big fan of small commits, often and breaking down changes but it does require a large amount of automation and tooling to support well.
Aren't "pull requests" a GitHub (and similar systems) thing? As far as I know, Git itself does not have a concept of "pull requests".
Of course, we still have giant PRs that touch 50-100 files and take forever to code review.
And yet, for those stories in question, they do make it to prod faster than if they were broken apart into multiple 1-2 point stories.
I imagine as always the answer is "invest in better tooling". More robust automated tests, push-button millisecond-deployments, etc...
I've only ever found this workflow to work if you can attach multiple commits/code reviews to single higher-level work items in your project management software. There's simply too much overhead if you try to correlate project management items one-to-one with commits/code reviews.
The only thing stacked PRs indicate, IMO, is that your coworkers are slow to review your code.
And in my experience, when folks send out stacked PRs, it’s because they put much _more_ thought and effort into identifying the right boundaries, not less.
First one is the API/interface with null implementations. Then subsequent ones each implement a method with associated tests
Large PRs preserve the larger "1000 feet view" of what you are working on but are likely to be slower to get responses on and most likely less thorough thus a larger chance of things being missed.
Almost everyone I've worked with prefers smaller review so I just accept that trade-off of 1
Bzr has no concept of rebasing, so what you get is a simple merge, but with very little work on your part.
Stacked PRs is _how_ you can break up your changes into small components to merge more quickly and block development less.
With stacked PRs the changes are broken into smaller pieces because a later PR can depend on an earlier PR. This means it can be developed while earlier PRs are still under review. It also means each review is smaller and therefore likely to complete more quickly.
Stacked PRs can also work like patch series in the Linux Kernel development where a large change is broken into smaller independent changes done in the logical order, even if their order of discovery was probably reverse. For example, if you discover you need a refactor or bug fix while halfway through developing a feature. In that case you simply insert a PR earlier in the stack/graph which contains just that isolated fix.
In your feature flag example, a common patten would be to have a dozen or so stacked PRs. The early ones do necessary refactors and bug fixes. The middle few add the feature flag and code. The last make the feature flag the default and delete the now obsolete !feature path. Then the prereqequites (which you didn't discover until half-way through) can be reviewed and merged quickly. The meat of the feature is broken up into easily digestible pieces for quick review, but reviewers can get a sense of the entire feature by considering the PR series as a whole; this is especially useful where one PR adds some infrastructure that a later PR uses but both PRs together would be too large to effectively review. As PRs are ready to merge, they can. Often the last couple of PRs which make the feature the default and delete the obsolete code do not merge for quite some time, but they were written when everything was still fresh in the developers head.
Currently, one can just do it by never modifying the underlying "branch" (i.e. just adding new commits), but in practice that doesn't always work, as many times you have to rebase against one's master/main branch to pull in changes, which even if no conflicts, will reflow your "base" branch" changing all the commits.
TLDR: Basically, want to be able to state that branch depends on branch (which is currently commit id x) but if I rebase, use whatever the current commit id is for that branch, not whatever it was when I first made the branch.
possible? stupid idea? thoughts?
Might be useful for you if you find the rebase flow to be painful. In particular this can be good for local prototyping where you have a bunch of functional candidates on top of some foundational refactors, and you may be rebasing frequently to refine those foundational commits.
- git sync: rebase all commit stacks on top of the main branch. - git restack: run after making a change to a foundational commit to automatically restack dependent commits.
You'd still end up fixing all the conflicts your branch made either way, and also breaking the flow of commits if you didn't update the message. So why bother doing all of that instead of stashing your current project, rebase, and force push it? I believe that's supposed to be the standard approach to handle conflict changes in PRs.
As far as I can tell (the “problem” you’re describing is a bit vague) this is already how git works with the sole exception being that git doesn’t default to a particular branch for a rebase?
eg) all you seem to de describing here is a bog-standard:
(on feature-branch1)> git checkout -b feature-branch2
(on feature-branch2)> git rebase feature-branch1
All it seems like you’re asking for us to drop “feature-branch1” from the rebase?
git branch ..., I'm creating a new branch pointer that points to a commit and any future commit will update that branch pointer (not the original).
if I then add a new commit to the original branch, I'll update its branch pointer.
if I do then do a git rebase -i original_branch (on new branch), (I believe) git will look for the common ancestor commit, update the head to original_branch's current id, and then apply individual every commit id from that common ancestor to the head of my what was my new branch.
if I just add a commit to original branch, this is "clean" (i.e. i might have lots of conflicts, but it makes sense, its just effectively inserting a new commit into the middle of the commit stream). However, if I have rebased the original branch, such that common ancestor isn't where it used to be, but much further back in time), it no longer makes sense.
What I was proposing (and what I could do manually), is when i rebase against the proposed type of branch, then we essentially, move the head to the current head of the updated "old branch" and then cherry-pick one by one each commit i made against the new branch (fixing merge errors along the way, just like with rebase).
is this a bit clearer?
[0] https://github.com/gitext-rs/git-stack
[1] https://github.com/gitext-rs/git-stack/blob/main/docs/compar...
https://docs.github.com/en/pull-requests/collaborating-with-...
https://docs.gitlab.com/ee/topics/git/git_rebase.html#rebase...