Comment by matharmin

5 hours ago

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

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)

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!

  • 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.

    • 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

      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/

      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.

  • > 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.

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.

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

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.