I don't want this. It's too complicated.
Plus it's vendor lock in.
Give it time. Soon you'll only be able to push using Github's CLI.
Stacked PRs are now live on GitHub
231–240 of 308 posts
Re: Stacked PRs are now live on GitHub
#232What's the benefit of this type of stacked PRs over a well-curated set of commits, and reviewing per commit? I think the bigger problem is that big AI PR's need a different way of reviewing. For example, the order in which the diff's are shown can make a big difference in how easy the commits are to read (e.g., function definition change first, then all call sites, then the tests). Or maybe we should go to a system w…
You can’t merge one commit at a time in a PR. In a stack, if the first 4 parts of a feature are good to go and there’s a problem with the 5th, the whole thing doesn’t need to be blocked.
> You can’t merge one commit at a time in a PR
Sure you can. The command is `git merge`
Re: Stacked PRs are now live on GitHub
#233I've been using the preview for a bit, and I'm quite surprised to see them expanding the preview with so many unfixed issue. For example, merging an entire stack is completely broken in many cases: https://github.com/github/gh-stack/discussions/212 You can merge one by one, but if you're using squash and merge, you need a re-approval for each PR in the stack if you require reviews. This makes you lose out on arguably…
It's been >4 years since I've been out of Facebook and now have to use Github, and to this day I miss phabricator and landing stacked changes. When I first left and had to work with github I hadn't realized that this, to me, very basic, workflow is _not_ a feature!
Re: Stacked PRs are now live on GitHub
#234Earlier quoted context omitted.
I get your point, but it kind of is AI related. The size of PRs since AI has made it much harder to review a diff. Stacking the PRs seems like a response to this problem.
We were benefiting from stacked pull requests in Phabricator for a decade before "AI" was a thing. Having well scoped commits that can be individually actioned by distinct sets of reviewers has always been extremely useful.
Meh. Stacked PRs (where a PR is already a stack of commits, and reviewers can perhaps communicate with each other to coordinate scope/responsibility) are a bit like tab groups in the browser when tabs, multiple windows, and workspaces already existed for maintaining organization. This new feature clearly scratches an itch for some users but creates more complexity (yes, even if you don't interact with it) for people already comfortable with the status quo.
Re: Stacked PRs are now live on GitHub
#235Nice. The pain for me was never creating the stack, it was keeping reviews and restacks inside Github.
Re: Stacked PRs are now live on GitHub
#236Earlier quoted context omitted.
> What's the benefit of this type of stacked PRs over a well-curated set of commits, and reviewing per commit? For the people who work with stacked diffs (in phab/otherwise) this is exactly what they'd consider reviewing a well-curated set of commits one-by-one. One distinction is that cognitively a unit of review (a PR, a diff) remains a single bound change. Comments are focused on that change and the PR does not gr…
IMO, in a team settings, improving the review policies and speed has a much better benefit. A PR is supposed to be a proposal for some change, adding more proposals on top of something that is not reviewed is a bit icky. > . By focusing the stack to the different reviewers you can avoid ambiguity about "what a person is signing off on" in the stack. That can be easily done with comments. If the PR are orthogonal, the…
This articulates a source of unease I was having trouble pinpointing. Encouraging piling up changes like this puts pressure on the reviewer to approve things as-is because "i've already built Y and Z on top of X so changing X now is a pain".
In my opinion there's already too much of a bias toward stamping PRs, and organizations that maintain high standards do it partly by resisting that bias in key parts of the codebase so people can push back on things that are poorly engineered or hard to maintain.
Re: Stacked PRs are now live on GitHub
#237I dislike them reinforcing the component approach to delivering work through their examples, like the top screenshot showing "database schema changes", "api changes" and "frontend implementation" as separate branches in a stack. So really, one does consider full stack a single feature, but unless they are reviewed in one go — which defeats the purpose of stacked branches and pull requests — you can end up landing one…
At Facebook (i left >4 years ago) we used Phabricator where shipping stacked changes like this was table stakes. I still miss it. Some few random points, not really organized, i'm sure i'm forgetting bunch more benefits:
* I often used it as a way to make reviews significantly easier for the reviewer. If I split something more complex into smaller units where the small change seems obviously correct, it's much more likely i'll get a faster accept than with a very large change.
* Critically, once a unit of the stack has been approved, the approval will still stay with rebases of the whole stack.
* You can keep building up a stacs and once you think you have enough for whatever you are doing, ship the whole stack.
* Suppose you have a 'hole' in the reviews and you're missing a review for a middle part of the stack. You can still merge the bottom part up to where all are reviewed.
* Let's assume you're the reviewer: smaller changes is much easier to review. Also, each part of the stack builds incrementally on the previous one. With your example, suppose you're reviewing 'api changes' part of the stack, but it has failing tests because something is broken with the lower 'db schema changes' change - but otherwise, the 'api changes' diff looks fine - you can then accept it and this part of the stack is already reviewed. The author can now whenever fix their bottom-most change (db schema change) to have tests passing, and get someone to review that (perhaps someone completely different more familiar with db), and then rebase the 'api changes' diff, and _it will not_ require review because you haven't changed anything, just updated the diff underneath it. In this way you parallelized your approvals. Right now at work, using github, I have to serialize my PRs and can't effectively get things reviewed until the previous PR is approved.
Re: Stacked PRs are now live on GitHub
#238Earlier quoted context omitted.
Here's a common flow where I find stacked PRs are useful: - I want to build feature X - Ah, but it would work better if I refactored the module first - I refactor then build feature X - There's then some additional (and optional) cleanup work As a reviewer I wouldn't want to see all this in a single PR, and the changes depend on each other so I can't open multiple independent PRs. Manual rebasing is fine but navigati…
Those could just be individual commits on a branch. Why does it need to be a stacked PR? That concept really only exists within the interfaces of these kinds of tools.
Yeah, I think it's largely a UI issue! I really want a UI that lets me review distinct changes that are dependent on each other, and handle comments/changes neatly within that structure. I've found GitHub quite lacking there historically.
Also, if you're the mchristen I know, then I think we've had this discussion before haha! Hope you're well :)
Re: Stacked PRs are now live on GitHub
#239Looking at the way GitHub are selling this “feature”, I feel like some of the engineers who are going to be excited about this feature for “reviewability” reasons are, in particular, those who’ve forgotten that they should be splitting changes into multiple logical commits inside a PR. And instead of that they’re now going to use multiple, single commit branches and stack them because stacked PRs are a “new” “feature…
Splitting changes into multiple commits is a much worse experience if you actually have self-contained dependent changes.
1. The whole review interface isn't set up for reviewing individual commits.
2. You can't merge changes progressively.
3. CI doesn't run on each commit.
4. If you have linear history (good idea IMO) you'll lose your nice commit history when you merge it.
This is much better.
Re: Stacked PRs are now live on GitHub
#240Honestly, I didn't even know this was a problem that needed to be solved.