Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow dismiss_reviews only on non-rebase updates #5052

Open
Sidnioulz opened this issue Nov 10, 2022 · 2 comments
Open

Allow dismiss_reviews only on non-rebase updates #5052

Sidnioulz opened this issue Nov 10, 2022 · 2 comments

Comments

@Sidnioulz
Copy link

Expected Behavior

An option for dismiss_review's when that only triggers when commits are added to a PR's head, but not when it is rebased (in other words, only accounting for changes made on commits not on the target branch).

Why? We use approvals on A/B branches to indicate that they have been reviewed, before triggering a release, which then allows other tools to run the A/B test down the line. We then want the reviews to be dismissed if the PR is updated with new changes, but we still want the branch to keep being released and used when the main branch is updated, so that it stays up to date.

With approvals dismissed on rebase, it's necessary to manually reapprove. If approvals could be dismissed only on non-rebase changes, our model could be implemented.

Actual Behavior

Not possible at the moment. Rebases trigger dismiss_review.

Steps to Reproduce the Problem

  1. Create config with dismiss_reviews and when set to either allowed value
  2. Open a PR, approve it
  3. Rebase the PR, notice approvals are gone

Specifications

  • Pull Request URL: ø
  • Mergify Config URL: ø
@jd
Copy link
Member

jd commented Nov 10, 2022

The main issue is that rebasing a branch changes all the sha1, so there's no way to know if the rebase introduced new commits, new content or not.

@Sidnioulz
Copy link
Author

Would it be acceptable to have it heuristics based? Do you have access to new/old diffs and messages, or only hashes, in this code path?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants