Skip to content
This repository has been archived by the owner on Apr 24, 2024. It is now read-only.

do not invalidate approved PRs after a rebase from master #59

Closed
wants to merge 2 commits into from

Conversation

ab77
Copy link
Member

@ab77 ab77 commented Mar 21, 2024

  • (hopefully) removing friction from PR approval flow, when an approved pull request becomes invalidated after a rebase from master

change-type: patch

* (hopefully) removing friction from PR approval flow, when an approved pull request becomes invalidated after a rebase from master

change-type: patch
@ab77 ab77 requested a review from klutchell March 21, 2024 22:36
@flowzone-app flowzone-app bot enabled auto-merge March 21, 2024 22:37
# "update merge" is a merge commit that was created in the UI or via the API
# and merges the target branch into the pull request branch. These are
# commonly created by using the "Update branch" button in the UI.
ignore_update_merges: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am quite sure that this will have no effect on rebase merges, where each commit is rewritten. Policy-bot is only identifying the literal merge commit from master mixed in with other PR commits, a process we avoid using and block via Flowzone.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure? From the description here, it may work (though I've not looked through their code).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is describing a merge commit (a commit with two parents) which is the same logic used for Flowzone to reject a PR for using the wrong type of master update. I am certain.

@klutchell
Copy link
Contributor

Approvals are only invalidated if the rebase is done by the approver, so they become an author/contributor and are no longer eligible for approvals.

We could deploy a bot to provide rebases via comments like we used to have via VersionBot.

Otherwise allowing contributors to approve leaves an opportunity for a compromised account to piggy-back changes on someone else's PR and self-approve.

@ab77
Copy link
Member Author

ab77 commented Mar 21, 2024

Approvals are only invalidated if the rebase is done by the approver, so they become an author/contributor and are no longer eligible for approvals.

We could deploy a bot to provide rebases via comments like we used to have via VersionBot.

Otherwise allowing contributors to approve leaves an opportunity for a compromised account to piggy-back changes on someone else's PR and self-approve.

Fair enough, then I guess we have a couple of options:

@ab77 ab77 closed this Apr 4, 2024
auto-merge was automatically disabled April 4, 2024 19:15

Pull request was closed

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

Successfully merging this pull request may close these issues.

2 participants