-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Land Whole Stack #45
Comments
You are 100% correct, it is possible. In fact, earlier version of stack-pr did land the full stack at once. There is one caveat - it doesn't work if there are pre-merge required checks enabled in the repo. When we land the whole stack, we need to still land the PRs one by one and rebase the remainder of the stack after each merge. This rebase will trigger a PR update, which will trigger the checks to restart, and that will block further landing (until checks are complete). In our repo we added such checks at some point, so full stack landing stopped working for us, and hence we updated the tool to only land the bottom PR from the stack. But it should be possible to (conditionally) restore the old behavior for repos that allow that. |
Ah, that makes a lot of sense. Supporting a check for if the PR is mergeable as a contingency for continuing to process the stack would hopefully be the best of both worlds. BTW, the 0.1.3 release has been working great so far 👍. |
There is a whole other alternative which I only briefly considered, but didn't go deep into details as it's not applicable in our repo: GH allows "rebase&land" method for landing PRs (the default on is "squash&merge"). With rebase&land it might be possible to land the entire stack at once even if there are required checks - in that case we will need to rewire the top PR and land it using that method. I don't know for sure if this would work (these rewirings can accidentally restart the checks, which would again block landing), but it can be an interesting avenue to explore as well. |
A good idea. Our repo also defaults to squash&merge to support the tradition github workflow of intermediate commits for changes so I haven't seen that landing strategy yet. My intuition is it would not work since only one PR is targeting main at a time in any given stack state (I assume rebase&merge is specific to a set of commits within a single PR). |
Right, the whole process would look something like this:
|
Ahhh, I see. Worth a shot. |
I had some spare time so I set up a test repository and investigated the rebase and merge strategy. I set up a string of commits including a test check that sleeps 15 seconds then prints the date. I used stack-pr to export the commits into a PR stack. I verified the check ran on the PR. Then I used the gh cli and changed the merge base of the HEAD PR to main. I verified that all commits from pervious PRs in the stacks were now included and that the check did not re-run. I then submitted the single PR via the gh cli with the rebase&merge strategy which worked as expected. TLDR: This strategy seems to work as desired. I think it's worth adding support into stack-pr as that would introduce an immense speed-up in regular workflow with any kind of regular checks such as running test suites. |
At present, the default
stack-pr land
and suggestedstack-pr land -B [branch-name]~[stack-count] -H [branch-name]
fromstack-pr view
leads me to believe these commands should land ALL entries in the stack. Instead, it processes the oldest entry in the stack and then stops. There are no errors and the local digraph state is correct. I'd expect either of the aforementioned land commands to land the whole stack rather than one at a time.The text was updated successfully, but these errors were encountered: