481–490 of 548 posts
Meanwhile, you still can't do fast-forward merges in GitHub :clown: https://github.com/orgs/community/discussions/4618 And it doesn't even rebase and merge correctly with fast-forward if there it's a clean set of commits! https://github.com/orgs/community/discussions/5524
[flagged]
Earlier quoted context omitted.
>If you reject change A and approve change B, nothing can merge The feature is also half done in this case. The author can fix up the concerns the reviewer had in A and then both can be merged at the same time.
Couldn’t they do that in one PR? Seriously, couldn’t you just say “hey Alice, could you review the A parts of this PR” and “hey Bob, could you review the B parts”, then only merge once both of them approve? Even GitHub, for all its faults, supports code owners files, such that this can even be policy.
As a counter example. Why use multiple PRs when you can always just merge them into a single one. It's possible to make huge PRs with a bunch of different changes all included, but then the GitHub tools with managing stuff don't really work that well and you have to just do everything as comments instead of being about to actually accept a single accepted change for example.
Earlier quoted context omitted.
I agree that a `gh stack` command is not needed, but this feels to me like just a better UI feature for a good git workflow. It literally is about making multiple smaller PRs that build on top of each other.
The question is, why are you not just merging them into main as you go? It's a bit of a smell when you "need" to merge branches into branches. It shows a lack of safety and ease in deployments, which is the real problem to solve IMO.
'Large pull requests are hard to review' is a good reason to keep PRs small, but small PRs also encourage continuous integration/delivery/deployment. Stacked PRs sound like they encourage long-lived feature branches instead.
I typically generate stacks of 3-5 PRs in 1-2 days now (in a gen-AI world).
Earlier quoted context omitted.
The teams that I have worked with still apply the philosophy you’re describing, but they consider PRs to be the “commit”, i.e. the smallest thing that is sane to apoly individually. Then the commits in the PR are not held to the standard of being acceptable to apply, and they are squashed together when the PR is merged. This allows for a work flow in which up until the PR is merged the “history of developing the PR”…
Doesn’t this mean that a first review might request that a specific change be reverted, and then a later reviewer reviews that reversion? That’s essentially reviewing a noop, but understanding the it’s a noop requires carefully checking all previous now-invalidated changes.
Earlier quoted context omitted.
We use this feature extensively at $dayjob. Imagine you have some task you are working on, and you wish to share your progress with people in bite sized chunks that they can review one at a time, but you also don’t want to wait for their reviews before you continue working on your task. Using a stacked set of PRs you can continue producing new work, which depends on the work you’ve already completed, without waiting…
You can review PR’s commit by commit, and you can ask people to review PR’s commit by commit. Not a big deal.
Earlier quoted context omitted.
The teams that I have worked with still apply the philosophy you’re describing, but they consider PRs to be the “commit”, i.e. the smallest thing that is sane to apoly individually. Then the commits in the PR are not held to the standard of being acceptable to apply, and they are squashed together when the PR is merged. This allows for a work flow in which up until the PR is merged the “history of developing the PR”…
Squashing is fine if you’re just making a mess of temporary commits as you work and you don’t want to keep any of those changes separate in master, but that’s not a useful review workflow. A lot of times I’ve built a feature in a way that decomposed naturally into e.g. two commits: one to do a preparatory refactor (which might have a lot of noisy and repetitive changes, like changing a function signature) and another…
Some examples of compromises:
You can’t merge partially merge a large “review commit by commit” PR so you are forced to wait until it is all ready to merge.
Meanwhile, you still can't do fast-forward merges in GitHub :clown: https://github.com/orgs/community/discussions/4618 And it doesn't even rebase and merge correctly with fast-forward if there it's a clean set of commits! https://github.com/orgs/community/discussions/5524
[flagged]
Earlier quoted context omitted.
It’s useful for large PRs in large repos with many contributors. It reduces the burden for reviewers.
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.
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.