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

Rebase invalidates approval #795

Open
paradisfm opened this issue Jul 9, 2024 · 1 comment
Open

Rebase invalidates approval #795

paradisfm opened this issue Jul 9, 2024 · 1 comment

Comments

@paradisfm
Copy link

When master updates between the time a PR is created and when it is merged, a rebase/"update master" commit is obviously required. However, if an approval is present at the time of the rebase, the rebase commit will invalidate the approve since it is a "new commit" despite the code in the PR not changing. Please investigate as this is quite cumbersome.

@bluekeyes
Copy link
Member

I've though about this before, and it's a challenging problem to solve in all cases. While as a PR author, it's clear that your rebase did not change anything, from Policy Bot's perspective, the commits on the PR are completely different after a rebase because Git generates new commit objects as part of the operation. These new commits may also contain different content (trees) in the case of automatically resolved merge conflicts. Auto-resolved conflicts are probably safe to accept, but it's hard to distinguish between those and manually resolved conflicts, which should be reviewed.

I have yet to find a way to verify all of this using the tools provided by the GitHub API, but if you have any ideas, I'm interested to hear them!

For now, here are some strategies that might minimize this problem:

  1. Consider if you actually need to require that branches are up to date before merging. Internally, we've decided that the (small) risk of semantic merge conflicts is an acceptable tradeoff to avoid the extra CI time and work required to keep PRs up to date, even in large monorepos with hundreds of contributors. But every organization will have a different opinion here, so I understand if it is not an option for you.
  2. Instead of rebasing, use GitHub's "Update branch" button to create a merge commit and then enable the ignore_update_merges option for the relevant rules
  3. Consider removing the invalidate_on_push requirement from rules, if allowed by your organization's security/compliance posture

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

No branches or pull requests

2 participants