Automatic change of base branch dismisses PR reviews

[ffissore]

Select Topic Area

Bug

Body

In order to create PRs focused on one particular kind of change, and thus easier to review, we are used to create what we call "PRs trains":

• PR from branch A wants to merge into main, PR from branch B wants to merge into A, and so on
• reviewers review each PR
• PR from branch A is merged, we wait until deployment goes well and all is green
• github automatically changes the base branch of PR from branch B
• PR from branch B is merged

Since this morning this flawless flow is no longer possible: my colleagues reviewed my PRs but when the first one was merged I got

and I had to ask my colleagues to reapprove.

Can you revert this change?

[stanfeldman]

This very annoying change breaks a typical workflow when you create incremental staggered PRs for a feature. We've got extra protection for a very unlikely scenario that inhibits some legit flows that are more common.
There are other people raising similar concerns.

[ffissore]

Hello github?

[ffissore]

I opened a support issue with github. The workaround is to disable/enable a couple of options in the branch protection rule

As a workaround, some customers have disabled the rule Dismiss stale pull request approvals when new commits are pushed and instead have enabled Require approval of the most recent reviewable push. These rules are slightly different - I'll describe them here:

Dismiss stale pull request approvals when new commits are pushed

Optionally, you can choose to dismiss stale pull request approvals when commits are pushed that affect the diff in the pull request. GitHub records the state of the diff at the point when a pull request is approved. This state represents the set of changes that the reviewer approved. If the diff changes from this state (for example, because a contributor pushes new changes to the pull request branch or clicks Update branch, or because a related pull request is merged into the target branch), the approving review is dismissed as stale, and the pull request cannot be merged until someone approves the work again

Require approval of the most recent reviewable push

Optionally, you can require that the most recent reviewable push must be approved by someone other than the person who pushed it. This means at least one other authorized reviewer has approved any changes. For example, the "last reviewer" can check that the latest set of changes incorporates feedback from other reviews, and does not add new, unreviewed content.

[mariux]

We experience the same issue but we can not use the provided workaround that @ffissore shared.

What is the expected timeline for a fix here as this blocks our workflows and frustrates our team ;)

Thanks for GitHub and the cewl features please make it work as awesome as before ;)

[alec-oms]

Getting the same problem here and is reducing team velocity and productivity.

Tried the work around and didn't work either. PR's ended up in a limbo state where review checks were complete, but still could not be merged without reverting the workaround, then closing and re-opening the PR.

Keen to find out when this will be fixed, or if there is another process/work around to achieve the same PR chaining approach.

[ffissore]

GH confirmed the suggestion above didn't aim at restoring the previous behaviour. They also said that at the moment the only way to have something similar is to disable Dismiss stale pull request approvals when new commits are pushed, which is a security concern and thus not applicable.
I'm afraid it's just broken.
Not sure who they talked to before deciding such a change, but I'd love to know what PR flows these github customers implement, because ours is broken and we have to harass each other more often asking for re-approvals

[joshuakeel]

Totally agree, this is affecting our team as well, and is really making our workflows more clunky.

[joshuakeel]

This setting is under the "General" section, and not "Branches", at least for me.

In addition, this setting is not turned on, and we are still having problems.

[pbs-jmo]

This isn't actually a solution. We should still be able to auto-prune branches and also keep the old behavior which was to not dismiss reviews as "stale" due to that happening.

[aaditmshah]

This is a very unfortunate regression. In my team, we introduced a new workflow. The person who reviews PRs is also responsible for merging them. They should merge the PRs as and when they are done reviewing them. That way, they don't need to re-approve PRs. Also, when PRs are marked as ready for review they are also ready to be merged. Thus, the reviewer can do their job asynchronously from other developers.

[aaditmshah]

[ffissore]

This is an interesting approach, thx @aaditmshah I'll give it a try

[davidkocnar]

What about chained PRs? You cannot review them as soon as their base is not develop and the base branch cannot be approved/merged.

[aaditmshah]

@davidkocnar For chained PRs you can use fast-forward merges. For example, suppose you have the following commit history.

A --- B --- C --- D --- E --- F --- G --- H --- I --- J --- K --- L --- M --- N --- O
            ^                 ^                 ^                 ^                 ^
            |                 |                 |                 |                 |
          main             develop           fix/foo         feature/bar       feature/baz

We have the following four PRs.

1. develop to main
2. fix/foo to develop
3. feature/bar to fix/foo
4. feature/baz to feature/bar

The reviewer can review and approve all the PRs using the GitHub UI. But when merging, the reviewer can merge the branches locally using fast-forward merges, and push the changes to GitHub.

git fetch # ensure that you have the latest changes locally
git checkout develop # you can run `git merge --ff-only origin/develop` to get the latest changes if required
git merge --ff-only fix/foo # merge the changes from `fix/foo` into `develop`
git push # you can even push to protected branches using fast-forward merges
git merge --ff-only feature/bar # now you can merge the changes from `feature/bar` into `develop`
git push # when using fast-forward merges you don't need to review the PRs again
git merge --ff-only feature/baz # finally you can merge the changes from `feature/baz` into `develop`
git push # github automatically marks the corresponding PRs as merged when using fast-forward merges
git checkout main # now we want to merge `develop` into `main`
git merge --ff-only develop # fast-forward merges can be done in any order
git push # everything can be done locally, without interacting with the github ui
git remote prune origin # after you are done remember to clean up the branches in your remote

If you use fast-forward merges frequently, like I do, then you might want to use the following git settings.

git config --global pull.rebase false
git config --global pull.ff only
git config --global merge.ff only

If you enable these settings then you don't need to pass the --ff-only flag manually.

[pbs-jmo]

This also breaks a common workflow for us at my company. This is pretty inconvenient.

[ChristopherGerdes]

This is also breaking our teams workflow. are there any plans to fix this?

I tried the suggestion of disabling "dismiss stale pull request approvals when new commits are pushed" and enable "require approval of most recent reviewable push" and it did not work.

[Geri-Borbas-Shape]

Hey Github, as a workaround: can't be re-approval requests automatically approved if there is no code change compared to the previous approval?

[ChristopherGerdes]

as an update to this thread - I filed an issue with GitHub and they told me it was by design. They said they would forward the request to their product team but I may not hear back.

sounds like this is unlikely to get fixed unfortunately. Too bad since this breaks workflows for a lot of people.

[tfrokt]

same here, really annoying "feature".

[forever425000]

We experience the same issue but we can not use the provided workaround that @ffissore shared.

What is the expected timeline for a fix here as this blocks our workflows and frustrates our team ;)

Thanks for GitHub and the cewl features please make it work as awesome as before ;)

[fcano-ut]

The GitHub interface is now inconsistent with itself. I'm getting PRs dismissed, ask reviewers to review again, and when they do they see in the interface that there are no changes to review. So I'm just asking them to approve an empty set of commits. I don't think it makes a lot of sense...

[Geri-Borbas-Shape]

So I'm just asking them to approve an empty set of commits.

Exactly ☝️, even only changing that could be a huge improvement (see https://github.com/orgs/community/discussions/57513#discussioncomment-7680274).