← Back to context

Comment by Gigachad

13 hours ago

Still not sure this is the right solution. My problem is if one of your first stages gets rejected in review or requires significant changes, it invalidates so much work that comes after it. I've always when possible preferred to get small stuff merged in to production as it happens rather than build an entire feature and put it up for review.

> it invalidates so much work that comes after it.

No, not necessarily.

I work on a large repo and new features often involve changes to 3 different services: 2 from the backend, and the frontend UI. Sending a single PR with changes to all 3 services is really not ideal: the total diff size in a feature I added recently was maybe 600+ lines, and the reviewers for frontend and backend changes are different people. The changes in the 2 backend services can be thought of as business logic on one side and interactions with external platforms on the other. The business logic can't work without integrating calls to external APIs, and the UI can't work without the business logic.

These days I open 3 separate PRs and the software only works once all 3 are merged and built. It would be great to have all of them as a single package that's still testable and reviewable as 3 distinct parts. The UI reviewer can check out the whole stacked PR and see it running locally with a functional backend, something that's not possible without a lot of manual work when we have 3 PRs.

The LLVM community used this model for years with Phabricator before it was EOL'd and moving to GH and PRs was forced. It's a proven model and works very well in complex code bases, multiple components and dependencies that can have very different reviewer groups. E.g: 1) A foundational change to the IR is the baseline commit 2) Then some tweaks on top to lay the groundwork for uses of that change 3) Some implementation of a new feature that uses the new IR change 4) A final change that flips the feature flag on to enable by default.

Each of these changes are dependent on the last. Without stacked PRs you have o only one PR and reviewing this is huge. Maybe thousands of lines of complex code. Worse, some reviewers only need to see some parts of it and not the rest.

Stacked diffs were a godsend and the LLVM community's number one complaint about moving to GitHub was losing this feature.