MaskRay wrote: > > Yes. This PR is stacked on top of 230020 . My > > [MaskRay/pr-shadow](https://github.com/MaskRay/pr-shadow/) doesn't support > > gh stack style stacks, but it doesn't require force pushes. > > spr also doesn't require force pushes and also doesn't include the diff from > downstack/upstack PRs. Getting this right I usually find pretty important for > my review experience.
I tried https://github.com/spacedentist/spr and gave up on it: - It adds a `Reviewers:` trailer to every commit message. `Reviewed-by:` was useful in the Phabricator days; `Reviewers:` is just noise. - It pushes branches to llvm/llvm-project instead of my fork, which @nikic has objected to. - When a PR lands via GitHub's "Squash and merge" instead of `spr land`, spr's internal state sometimes gets out of sync. My pr-shadow pushes to my fork, and that is what causes the extra diff. GitHub requires a PR's base branch to be in the upstream repository, but #230020's branch is `MaskRay:pr/passes-options`. So this PR has to target `main`, and its diff includes #230020's changes. I usually avoid stacked PRs. I meant to open this one after #230020 merged, but I put it up early to show the migration path for `-print-pipeline-passes`. Until #230020 lands, the commit list shows what this PR adds: the first commit is #230020, and the rest is this PR's change. As for `gh stack`, its force-push workflow has three problems: - The timeline says "force-pushed the BB branch from X to Y", and "Compare" shows `git diff X..Y`. That diff includes every upstream commit in between, not just the change to the patch. With 100+ commits landing in LLVM every day, the comparison is useless. - Inline comments can become outdated or attach to the wrong lines. - If the commit message mentions an issue or PR, every force push adds another cross-reference to that page. Wrapping the reference in backticks avoids this, but it's a workaround. Long term, I think we should move off GitHub, which would also cut down on LLM spam. https://github.com/llvm/llvm-project/pull/230220 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
