4 ms·

At Facebook (i left >4 years ago) we used Phabricator where shipping stacked changes like this was table stakes. I still miss it. Some few random points, not re

At Facebook (i left >4 years ago) we used Phabricator where shipping stacked changes like this was table stakes. I still miss it. Some few random points, not really organized, i'm sure i'm forgetting bunch more benefits: * I often used it as a way to make reviews significantly easier for the reviewer. If I split something more complex into smaller units where the small change seems obviously correct, it's much more likely i'll get a faster accept than with a very large change. * Critically, once a unit of the stack has been approved, the approval will still stay with rebases of the whole stack. * You can keep building up a stacs and once you think you have enough for whatever you are doing, ship the whole stack. * Suppose you have a 'hole' in the reviews and you're missing a review for a middle part of the stack. You can still merge the bottom part up to where all are reviewed. * Let's assume you're the reviewer: smaller changes is much easier to review. Also, each part of the stack builds incrementally on the previous one. With your example, suppose you're reviewing 'api changes' part of the stack, but it has failing tests because something is broken with the lower 'db schema changes' change - but otherwise, the 'api changes' diff looks fine - you can then accept it and this part of the stack is already reviewed. The author can now whenever fix their bottom-most change (db schema change) to have tests passing, and get someone to review that (perhaps someone completely different more familiar with db), and then rebase the 'api changes' diff, and _it will not_ require review because you haven't changed anything, just updated the diff underneath it. In this way you parallelized your approvals. Right now at work, using github, I have to serialize my PRs and can't effectively get things reviewed until the previous PR is approved.

necovek 2mo ago
As I mentioned, this is similar to a workflow we used at Canonical ~15 years ago with bzr-pipeline plugin for bzr. Except that you tried not to divide it by technical aspect (DB changes vs API vs frontend) bit instead into smaller, semantic full features (we also used a 800/400 line diff limit which helped enforce it).
alepar 2mo ago
> it's much more likely i'll get a faster accept than with a very large change. I do agree the above is true. And maybe it's just me, but it feels like I'm kind of cheating if I stack PRs to get a faster approval. Because when one reviews some dangling code without the context of how it's going to be wired in, they can't really tell what the blast radius will be and consequently what bar they should hold the code against (e.g. performance, security, etc). In these circumstances, usually the most one can do is raise some stylistic nits (which to be fair PR process often degrades into anyway) and move on.

necovek 2mo ago

As I mentioned, this is similar to a workflow we used at Canonical ~15 years ago with bzr-pipeline plugin for bzr. Except that you tried not to divide it by technical aspect (DB changes vs API vs frontend) bit instead into smaller, semantic full features (we also used a 800/400 line diff limit which helped enforce it).

alepar 2mo ago

> it's much more likely i'll get a faster accept than with a very large change. I do agree the above is true. And maybe it's just me, but it feels like I'm kind of cheating if I stack PRs to get a faster approval. Because when one reviews some dangling code without the context of how it's going to be wired in, they can't really tell what the blast radius will be and consequently what bar they should hold the code against (e.g. performance, security, etc). In these circumstances, usually the most one can do is raise some stylistic nits (which to be fair PR process often degrades into anyway) and move on.