PR's Approval should be revoked whenever code changes are force pushed
Closed this issue · 1 comments
The following scenario is problematic:
- A PR is approved
- CI may or may not complete successfully
- The developer makes code changes
- The developer force pushes the changes
- The PR is merged since the force pushed branch is undergoing a minimal set of checks, not full CI
The force pushing erases the history of the CI that ran on the branch (that may have failed). However, since the PR is both approved and passes the minimal set of checks after the forced push, merging is enabled.
What should happen is:
A PR may not be allowed to merge unless it has passed full CI on the latest code that is to be merged. So, if a force push occurs that contains code changes, full CI must be initiated and merging should be disabled. This could be achieved by revoking the approval and requiring the reviewer to re-approve.
There are cases where force pushing contains no code changes. For example, amending the commit message or squashing commits.
Regardless, force pushing should not erase the CI that ran previously so that force pushing even without code changes should not allow a PR to be merged if it hasn't successfully completed CI (and it's best if the logs of the last CI were kept somehow)
"Dismiss stale pull request approvals when new commits are pushed" was activated.