[7/7][sl][github][gh stacks] update some documentation - #1398
Open
raydatray wants to merge 7 commits into
Open
Conversation
This was referenced Jul 31, 2026
|
This pull request has been imported. If you are a Meta employee, you can view this in D114402784. (Because this pull request was imported automatically, there will not be any future comments.) |
|
@raydatray has updated the pull request. You must reimport the pull request before landing. |
### ctx
github added native stacked pull requests, it would be nice to add native support to it for sapling!
this stack lets `sl pr submit` and `sl pull` via a new `github.pr.workflow = stacked` mode
### changes made
- `github_gh_cli.make_request` accepts custom headers (`-H`, needed for the `X-GitHub-Api-Version` preview header)
- `_format_param` supports list values using the `gh api` repeated-field syntax (`pull_requests[]=101`)
- an empty list is passed explicitly as `key[]` without a value (per the gh api manual) rather than dropped, since an empty array and a missing field can mean different things to an endpoint
- mock_utils: `MockGitHubServer` now tracks which expectations were consumed; tests can opt in via `wrap_with_consumption_check` to fail when an expected request silently stops happening (closes the old TODO)
### test plan
doctests in `_format_param` (registered in test-doctest.py, including the empty-array case), existing github .t suite. verified the empty-array wire format empirically: `gh api -F "pull_requests[]" --verbose` sends `{"pull_requests": []}`
### ctx basic plumbing for github native stacks REST API ### changes made - `gh_submit`: `StackDetails` dataclass + `get_stack_for_pull_request`, `create_stack`, `add_prs_to_stack`, `unstack`, all pinned to the `2026-03-10` preview API version - `unstack` returns the remaining stack when dissolution is partial (merged/queued diffs cannot be unstacked) so callers can react instead of assuming success - `update_pull_request` now takes `base: Optional[str]`: github rejects `updatePullRequest` mutations that include `baseRefName` for PRs in a native stack, so `base=None` uses a new mutation variant (`GRAPHQL_UPDATE_PULL_REQUEST_NO_BASE`) that only touches title/body ### test plan CI (doctest on `_parse_stack_from_dict`), and dogfooded on my own computer via a patch raydatray/rusty-mcrouter#198 <img width="905" height="468" alt="Screenshot 2026-07-31 at 3 20 04 PM" src="https://github.com/user-attachments/assets/bbe461a5-4128-4c75-b603-8ef4e012bb05" />
### ctx introduces the new native github stack workflow ### changes made - new `SubmitWorkflow.STACKED` variant, selected via `github.pr-workflow=stacked` - chains each PR's base to the head branch of the PR below it (like `single`), factored into `SubmitWorkflow.uses_chained_bases()` - hardened chained-base selection everywhere it happens (base-update loop, body rewrites, serial and placeholder creation): closed/merged diffs are skipped when picking the base below (their head branches would break the chain), and forks never chain (fork head branches cannot be bases on the upstream repo, so fall back to the default branch) ### test plan - new test-ext-github-pr-submit-stacked.t (initial submit) - test-ext-github-pr-submit-closed.t: open diffs stacked on a closed one chain past it to main - test-ext-github-pr-submit-placeholder-issue.t: placeholder strategy on a fork creates diffs against the upstream default branch
### ctx github renders native stacks in the PR UI itself so the sapling footer would be redundant - lets remove it when submitting via native stacks ### changes made - `create_pull_request_title_and_body` takes a `stack_list` flag; the stacked workflow omits the footer, other workflows are unchanged ### test plan updated mocks + test-ext-github-pr-submit-stacked.t
### ctx actually links the submitted PRs into a native github stack and deals with the various constraints github puts on stacked diffs ### changes made - after submit, diffs are linked via the stacks API: create the stack on first submit, then append new diffs when the local stack extends it at the top - the remote stack is queried up front, before any mutations, and submit fails closed: - if the query fails, abort (pushing blind against an unknown stack state could corrupt it) - if it diverged from the local stack (reorder, dropped diffs, or a new diff inserted below the top - stacks can only grow at the top), abort before updating bases or pushing. new `--restack` flag dissolves and recreates it (since the gh API has no reorder operation). tell the user to do this as a ui hint - `--restack` refuses to dissolve a stack it could not recreate (fewer than 2 diffs), and aborts if dissolution is only partial (merged/queued diffs stay behind) - closed stacks are treated as no stack - base branches of diffs already in a stack are never updated via the API (since gh rejects that, the stack manages bases on its own). body/title rewrites use the "base-less" mutation from facebook#1393 - new diffs are created directly against the head branch below them instead of being created against main and re-based afterwards (this is impossible to do once diffs are stacked on gh). falls back to default base for forks, where gh native stacks are not supported and a warning is printed (i wanted to submit this stack with my patch to show it works but alas i cant :( ) ### test plan - test-ext-github-pr-submit-stacked.t covers: the initial submit, extending an existing stack, diverged stack with/without --restack and with/without local changes to push, mid-stack insertion with/without --restack, stacks API query failure, partial unstack (abort up front / warn during sync), --restack with fewer than 2 diffs, closed stack, fork fallback - test-ext-github-pr-submit-stacked-placeholder.t: placeholder strategy + stacked - new mocks derive commit hashes from the test repo at runtime (reposetup) instead of hardcoding them, and use the consumption check from facebook#1392 so unexercised expectations fail the test - tested live on my own machine/repo ``` sl config --local extensions.github=/Users/ray/sapling/eden/scm/sapling/ext/github/__init__.py sl config --local extensions.signing_shim=/Users/ray/.sl-signing-shim.py sl config --local github.pr-workflow=stacked ``` - create a stack here raydatray/rusty-mcrouter#197 - submit 197 and 198 - it creates a stack - then add 200 locally and submit again - it extends the stack - then add 202 locally and submit again - it extends the stack - go down to 198 and amend the diff locally. restack locally and resubmit - modifies only 198 and the stack is updated <img width="906" height="465" alt="Screenshot 2026-07-31 at 3 42 08 PM" src="https://github.com/user-attachments/assets/16287bdb-fafa-4bf9-9427-5265875f9acf" />
### ctx `sl pr pull` links a diff's ancestors by parsing the sapling stack list footer, which the stacked workflow omits in facebook#1395 ### changes made - when the body has no footer, query the native stacks API and link the ancestors below the diff from the stack instead - warns "no stack information" only if the diff is in no stack at all; a diff that is in a stack but no longer an open member of it (e.g. merged - the API only lists open members, so its position is unknown) gets a specific warning instead of the misleading one - pulling the bottom of a stack links nothing and warns nothing ### test plan - new test-ext-github-pr-pull-stacked.t with mocked stack responses: top of stack, bottom of stack, merged member - dogfooded on my own computer ``` sl config --local extensions.github=/Users/ray/sapling/eden/scm/sapling/ext/github/__init__.py sl config --local extensions.signing_shim=/Users/ray/.sl-signing-shim.py sl config --local github.pr-workflow=stacked ``` - create a stack here raydatray/rusty-mcrouter#197 - rebase and merge as stack 197 and 198 - they merge as a stack and github doenst ask you to rebase (like it normally would in the previous pr workflow) <img width="919" height="504" alt="Screenshot 2026-07-31 at 3 49 38 PM" src="https://github.com/user-attachments/assets/f96fbc26-6d14-4229-a1e1-11cc4efb1a5b" /> - go back to local and run sl pull - we see what those two are marked as landed <img width="642" height="262" alt="Screenshot 2026-07-31 at 3 50 14 PM" src="https://github.com/user-attachments/assets/ee09c12d-0ea0-426c-8543-f19376436d29" /> - rebase those two remaining diffs on main, and then resubmit, we see that the stack remains preserved on github <img width="901" height="525" alt="Screenshot 2026-07-31 at 3 51 26 PM" src="https://github.com/user-attachments/assets/585dcb8b-b60d-4513-bc4e-96bb7705d08c" />
### ctx documentation updates for the new native stacked workflow ### changes made - extension help text for `github.pr-workflow` (overlap/single/stacked) and `pr submit --restack` - website: github.md + sapling-stack.md updated to describe the stacked workflow and its tradeoffs ### test plan docs only
|
@raydatray has updated the pull request. You must reimport the pull request before landing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ctx
documentation updates for the new native stacked workflow
changes made
github.pr-workflow(overlap/single/stacked) andpr submit --restacktest plan
docs only
Stack created with Sapling. Best reviewed with ReviewStack.