Chained / stacked PR process not working with recent approval rule changes #57944
Replies: 4 comments
-
This is a massive regression. It totally removes any benefit of raising stacked PRs... there's no point in getting reviews for 2nd and subsequent PRs if they're just going to be summarily discarded when the 1st one is merged. Please consider reverting this. |
Beta Was this translation helpful? Give feedback.
-
What is the status on this? The current behavior incentivizes large PRs which in turn lead to weaker review quality (overwhelming size of PRs) and thus deteriorated code quality. Note: Merging the other way around, e.g. PR3 -> PR2 -> PR1 (into master), is also not desired because it squashes all stacked PRs into a single commit, and PRs can not be reverted individually or cherry-picked for releases. This is a regression that leads to deteriorated UX and deteriorated code quality. |
Beta Was this translation helpful? Give feedback.
-
This breaks our teams workflow as well, and we are specifically trying to encourage people to move to smaller PRs. Is there any intent to fix this? |
Beta Was this translation helpful? Give feedback.
-
🕒 Discussion Activity Reminder 🕒 This Discussion has been labeled as dormant by an automated system for having no activity in the last 60 days. Please consider one the following actions: 1️⃣ Close as Out of Date: If the topic is no longer relevant, close the Discussion as 2️⃣ Provide More Information: Share additional details or context — or let the community know if you've found a solution on your own. 3️⃣ Mark a Reply as Answer: If your question has been answered by a reply, mark the most helpful reply as the solution. Note: This dormant notification will only apply to Discussions with the Thank you for helping bring this Discussion to a resolution! 💬 |
Beta Was this translation helpful? Give feedback.
-
Select Topic Area
Question
Body
Our team uses a workflow of chained/stacked PRs, alongside branch protection and codeownership.
Often, we'll create a chain of PRs for a feature A, with different reviewers assigned to each step in the chain:
PR 1: main <- a/one
PR 2: a/one <- a/two
PR 2: a/two <- a/three
...
and so on
Previously, if we had a reviewer approve PR 2, and subsequently, another reviewer approved PR 1, the authors or reviewers could merge PR 1, then merge PR2 (the approval for PR2 would persist).
Now, it seems that merging PR 1 will dismiss the approval on PR 2, requiring us to either merge PR1 and PR2 together before merging to master (not desired and not always possible for deployment scheduling reasons); or to reapprove PR2 (might take 24+hours due to timezone skew, so also not acceptable).
It seems like these changes may be part of this announcement: https://github.blog/changelog/2023-06-06-security-enhancements-to-required-approvals-on-pull-requests/
Is this an intended change?
If it is intended, is there any way to disable the behavior where merging a base PR dismisses the approval on a tip PR, without removing other branch protection rules (for instance, we don't want to disable the dismiss stale approval setting)? Ie, is there a way to restore the old behavior.
This is a critical workflow for our team, who work across multiple timezones.
Beta Was this translation helpful? Give feedback.
All reactions