4 ms·

I'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 sta

I'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 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 the biggest gain of stacked PRs. The command line tooling (gh stack) helps to make things slightly less manual, but you still need to be very aware of how git rebase works, the tooling just helps automate it across multiple branches. For example, just running the "gh stack rebase" commands that the UI suggests won't work if your local branches are not in sync with the remote ones, and the tooling won't point that out to you. I do find the stack UI quite nice. It's quite minimal compared to standalone PRs, but it's enough to show the relationship between them. (My comments all assume you already have a good reason to stack PRs. This tooling just help to make the workflow easier, it does not give any new capabilities)

sameenkarim 2mo ago
We're rolling out a series of bug fixes for the issues with squash merging. There's an internal system we have called CPRMC (Create Pull Request Merge Commit) that is used to evaluate whether a PR is "ready" to merge. This covers everything from mergeability (checking for merge conflicts) to rule evaluations (ensuring that approvals match the potential commit that will be created by merge) and more. This becomes particularly difficult when squash merging a stack of multiple PRs because we have to calculate a series of squashed commits, then associate those back to the rules/reviews. This is relatively easy for the first PR, but for the second PR onwards this gets more complicated because the ancestor commits are squashed and don't exist on the branch as-is. And I won't get into how much more complicated it gets for multi-parent situations lol. It's something we need to fix and it's the top priority for the team. Our numbers show that 99% of stack merges go through successfully, but we need to get that much higher. Thank you for being an early user in the preview and bearing with us while we work out these issues!
- masklinn 2mo ago
  > There's an internal system we have called CPRMC (Create Pull Request Merge Commit) that is used to evaluate whether a PR is "ready" to merge. This covers everything from mergeability (checking for merge conflicts) to rule evaluations (ensuring that approvals match the potential commit that will be created by merge) and more. By the way could there be a way to disable that when doing integrations externally? It seems to be quite costly (which makes sense), and the pull/ refs kinda bloat the reflist. I’m sure that external integration is not exactly beloved internally but there’s really just a small handful of big annoyances which would make it so much nicer and more comfortable.
- Game_Ender 2mo ago
  Can you make sure there is good API support for stacks? We use a custom merge queue and we want it to be able to land multiple PRs from a stack at once as separate PRs. Last I checked you had to land a single PR, rebase the stack, land the next and so on. This is very expensive in CI time (and wall clock time), vs simply testing part or all of a stack in parallel then declaring those merged. In essence a robot needs the ability to say “squash merge these 3 stacked PRs”, after the queue does its thing.
  - sameenkarim 2mo ago
    Yes that was one of our top priorities. For one, there's a fully public API for all stack operations. So if you don't want to use the `gh stack` CLI, you can build your own: https://docs.github.com/en/rest/pulls/stacks https://docs.github.com/en/rest/pulls/stacks For merging, we have an API but had to move it to a new async method: https://github.github.io/gh-stack/reference/merge-api/ https://github.github.io/gh-stack/reference/merge-api/ The legacy API was fully synchronous, and since stacks of multiple PRs can often take more than 10s (our global timeout), we had to move to async. We've had some folks already use this to integrate stacks into their merge queues. The great part is you can land multiple PRs in one atomic operation, and then there's one push to main with all your commits from multiple PRs. So instead of having to rerun the build/deploy for each, it can trigger for the last commit that contains all of the changes.
selimthegrim 2mo ago
what if you stack a draft PR on top of a regular one?
- sameenkarim 2mo ago
  The draft one can't be merged, but the one below it isn't affected and can be merged as long as it's green. The dependency goes bottom-up.
ransom1538 2mo ago
If you thought git was complex, wait until you get your 5 deep stacked PR from that nice coworker! Jokes aside this is really an anti-feature. Now coworkers can blow off your PR for much longer.
keeganpoppen 2mo ago
hahahah yeah my company had sooooo many issues with this stuff and merge queue lately
vermilingua 2mo ago
You’re surprised? Really? I’m more surprised that they pushed a major change and the site is still up.
an0malous 2mo ago
the whole industry has become completely “ready, fire, aim” since 2021
- mosura 2mo ago
  GitHub has been especially bad. Ultimately they have become a monopoly so it is kind of expected.
  - zelphirkalt 2mo ago
    So many things on Github are now broken or utterly unusable, that this comment is not wrong. I mean even the code viewer/display is utterly broken and doesn't even perform standard text highlighting correctly. The folding marks often make no sense or are inconsistent. If one wants to see all forks of a repo, one has to go through "insights", I think few would suspect it to be located, and it doesn't show _all_ forks, according to the number displayed next to the fork button. The list of UI issues goes on and on. But then there are also the reliability issues, things being down every 2 weeks, or is it every week by now? Difficult to keep up with it really.
  - OtomotO 2mo ago
    They are a monopoly in certain regions of the world. Even though I live in a western country, just one of my clients after all these years of being self-employed, uses GitHub. Nobody else would ever use it. And why would they? Especially for proprietary applications.
    - CalRobert 2mo ago
      That's interesting, I get pushback when I suggest anything different and for devs under 30 I've had to remind them that git and github aren't the same thing. This is not to cast aspersions on them, just the state of things in the places I've worked. Microsoft seems to be running the classic embrace, extend, extinguish playbook on github/git though. When it becomes impractical to join a team's workflow without github specific tooling then everyone will be forced to use gh.
hobofan 2mo ago
We just today ran into a bug where if the branch a stacked PR points to is deleted, it can just stay stuck in "merging" without any additional feedback. I directly looked at the GH status page, because who knows, maybe the PR subsystem has a partial outage again, but no, it was just a bug in the stacked PR feature.
- sameenkarim 2mo ago
  Thanks, definitely sounds like a bug. Will look into this.
saghm 2mo ago
> For example, merging an entire stack is completely broken in many cases: https://github.com/github/gh-stack/discussions/212 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 the biggest gain of stacked PRs. I'm struggling to imagine what it offers at all if that doesn't work! Is it just a way of manually marking another MR as a dependency in the UI so that it shows up with a red X if the other one isn't merged yet?
- Filligree 2mo ago
  Not everybody uses squash and merge. Regular merge, I imagine, would work fine.
  - saghm 2mo ago
    Sure, I just think that "merging one by one" defeats the purpose of stacked PRs even if you don't need re-approval. You can already avoid re-approval today without needing re-approval if you're not rebasing (which is not what I'd expect you mean by "regular merging"; it sounds like you mean just having a merge commit) by just not clicking merge until the other PR merges first, and at that point. That's what I meant by it sounding like all this offers is an icon telling you whether that other one already merged or not.
progval 2mo ago
Stacked PRs are so hard to implement for software not natively designed for them. Gitlab has been struggling with them too, despite 2.5 years since the MVP (https://gitlab.com/gitlab-org/cli/-/work_items/7473 https://gitlab.com/gitlab-org/cli/-/work_items/7473) . It doesn't help they want to do it without making any significant change outside the CLI, or that most developpers just... don't get it.
radicality 2mo ago
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!

sameenkarim 2mo ago

We're rolling out a series of bug fixes for the issues with squash merging. There's an internal system we have called CPRMC (Create Pull Request Merge Commit) that is used to evaluate whether a PR is "ready" to merge. This covers everything from mergeability (checking for merge conflicts) to rule evaluations (ensuring that approvals match the potential commit that will be created by merge) and more. This becomes particularly difficult when squash merging a stack of multiple PRs because we have to calculate a series of squashed commits, then associate those back to the rules/reviews. This is relatively easy for the first PR, but for the second PR onwards this gets more complicated because the ancestor commits are squashed and don't exist on the branch as-is. And I won't get into how much more complicated it gets for multi-parent situations lol. It's something we need to fix and it's the top priority for the team. Our numbers show that 99% of stack merges go through successfully, but we need to get that much higher. Thank you for being an early user in the preview and bearing with us while we work out these issues!

masklinn 2mo ago
> There's an internal system we have called CPRMC (Create Pull Request Merge Commit) that is used to evaluate whether a PR is "ready" to merge. This covers everything from mergeability (checking for merge conflicts) to rule evaluations (ensuring that approvals match the potential commit that will be created by merge) and more. By the way could there be a way to disable that when doing integrations externally? It seems to be quite costly (which makes sense), and the pull/ refs kinda bloat the reflist. I’m sure that external integration is not exactly beloved internally but there’s really just a small handful of big annoyances which would make it so much nicer and more comfortable.
Game_Ender 2mo ago
Can you make sure there is good API support for stacks? We use a custom merge queue and we want it to be able to land multiple PRs from a stack at once as separate PRs. Last I checked you had to land a single PR, rebase the stack, land the next and so on. This is very expensive in CI time (and wall clock time), vs simply testing part or all of a stack in parallel then declaring those merged. In essence a robot needs the ability to say “squash merge these 3 stacked PRs”, after the queue does its thing.
- sameenkarim 2mo ago
  Yes that was one of our top priorities. For one, there's a fully public API for all stack operations. So if you don't want to use the `gh stack` CLI, you can build your own: https://docs.github.com/en/rest/pulls/stacks https://docs.github.com/en/rest/pulls/stacks For merging, we have an API but had to move it to a new async method: https://github.github.io/gh-stack/reference/merge-api/ https://github.github.io/gh-stack/reference/merge-api/ The legacy API was fully synchronous, and since stacks of multiple PRs can often take more than 10s (our global timeout), we had to move to async. We've had some folks already use this to integrate stacks into their merge queues. The great part is you can land multiple PRs in one atomic operation, and then there's one push to main with all your commits from multiple PRs. So instead of having to rerun the build/deploy for each, it can trigger for the last commit that contains all of the changes.

selimthegrim 2mo ago

what if you stack a draft PR on top of a regular one?

sameenkarim 2mo ago
The draft one can't be merged, but the one below it isn't affected and can be merged as long as it's green. The dependency goes bottom-up.

ransom1538 2mo ago

If you thought git was complex, wait until you get your 5 deep stacked PR from that nice coworker! Jokes aside this is really an anti-feature. Now coworkers can blow off your PR for much longer.

keeganpoppen 2mo ago

hahahah yeah my company had sooooo many issues with this stuff and merge queue lately

vermilingua 2mo ago

You’re surprised? Really? I’m more surprised that they pushed a major change and the site is still up.

an0malous 2mo ago

the whole industry has become completely “ready, fire, aim” since 2021

mosura 2mo ago
GitHub has been especially bad. Ultimately they have become a monopoly so it is kind of expected.
- zelphirkalt 2mo ago
  So many things on Github are now broken or utterly unusable, that this comment is not wrong. I mean even the code viewer/display is utterly broken and doesn't even perform standard text highlighting correctly. The folding marks often make no sense or are inconsistent. If one wants to see all forks of a repo, one has to go through "insights", I think few would suspect it to be located, and it doesn't show _all_ forks, according to the number displayed next to the fork button. The list of UI issues goes on and on. But then there are also the reliability issues, things being down every 2 weeks, or is it every week by now? Difficult to keep up with it really.
- OtomotO 2mo ago
  They are a monopoly in certain regions of the world. Even though I live in a western country, just one of my clients after all these years of being self-employed, uses GitHub. Nobody else would ever use it. And why would they? Especially for proprietary applications.
  - CalRobert 2mo ago
    That's interesting, I get pushback when I suggest anything different and for devs under 30 I've had to remind them that git and github aren't the same thing. This is not to cast aspersions on them, just the state of things in the places I've worked. Microsoft seems to be running the classic embrace, extend, extinguish playbook on github/git though. When it becomes impractical to join a team's workflow without github specific tooling then everyone will be forced to use gh.

hobofan 2mo ago

We just today ran into a bug where if the branch a stacked PR points to is deleted, it can just stay stuck in "merging" without any additional feedback. I directly looked at the GH status page, because who knows, maybe the PR subsystem has a partial outage again, but no, it was just a bug in the stacked PR feature.

sameenkarim 2mo ago
Thanks, definitely sounds like a bug. Will look into this.

saghm 2mo ago

> For example, merging an entire stack is completely broken in many cases: https://github.com/github/gh-stack/discussions/212 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 the biggest gain of stacked PRs. I'm struggling to imagine what it offers at all if that doesn't work! Is it just a way of manually marking another MR as a dependency in the UI so that it shows up with a red X if the other one isn't merged yet?

Filligree 2mo ago
Not everybody uses squash and merge. Regular merge, I imagine, would work fine.
- saghm 2mo ago
  Sure, I just think that "merging one by one" defeats the purpose of stacked PRs even if you don't need re-approval. You can already avoid re-approval today without needing re-approval if you're not rebasing (which is not what I'd expect you mean by "regular merging"; it sounds like you mean just having a merge commit) by just not clicking merge until the other PR merges first, and at that point. That's what I meant by it sounding like all this offers is an icon telling you whether that other one already merged or not.

progval 2mo ago

Stacked PRs are so hard to implement for software not natively designed for them. Gitlab has been struggling with them too, despite 2.5 years since the MVP (https://gitlab.com/gitlab-org/cli/-/work_items/7473 https://gitlab.com/gitlab-org/cli/-/work_items/7473) . It doesn't help they want to do it without making any significant change outside the CLI, or that most developpers just... don't get it.

radicality 2mo ago

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!