Does not unapprove if target branch has changes, but plugin states it does.

Issue #20 wontfix
Edward created an issue

When installing the plugin I see:

text.png

However there is no option to enable it for committing to the target branch. This should be added as an option.

Comments (5)

  1. Douglas Gnoato

    Commenting on behalf of a customer.

    We are in a situation where there are a bunch of PRs open at a time (nothing unusual here) and we trying to use the PRs as gates for when to merge. We are using the following criteria:

    • Minimum number of approvers
    • Successful build of a \"temp\" merge of the PR and target branch
    • No outstanding tasks

    The issue is that even if the PR is ready to be merged it may stay open for undefined amount of time – could be an hour could be several days. In the meantime the target branch could have changed(could have multiple changes from other commits), but the merge button on the PR is not disabled and the PR could be merged thus breaking the target branch.

    We used to build all PRs automatically when there was change in target branch and if a PR build failed the merge was disabled. This is not a good way to handle the use case and it also doesn't scale well so we are looking ways to solve this.

  2. Bryan Turner Account Deactivated

    "target branch is changed" does not mean new commits are added to the target branch. It means the pull request is edited to target a new branch, and that branch isn't referring to the same commit as the previous target.

    In other words, if a pull request is targeting release/6.0, which is at commit abcdef1234, and someone else merges a pull request that updates release/6.0 to "123456abcd", pull request approvals are not removed because the target branch has been updated but has not been changed.

    On the other hand, if a pull request is targeting release/6.0 and someone edits the pull request and changes the target to master, then: * If release/6.0 and master are at the same commit, approvals are not removed * If release/6.0 and master are at different commits, approvals are removed

    Perhaps the documentation for the app needs to be updated, but it's working as intended--and the way it's working will not be changed. Target branches, especially "hot" branches like master, can be updated extremely often, especially in busy repositories used by large teams. Calculating whether or not to remove approvals on every target branch change for such repositories would get extremely expensive, especially for teams that tend to have a lot of pull requests open at once.

  3. Bryan Turner Account Deactivated

    The current behavior is working as intended. A target branch "change" means when the pull request is updated to target a new branch, not when the existing target branch is updated with new commits.

  4. Edward reporter

    Thanks Bryan for the detailed information, Makes more sense now. I believe this was raised on behalf of a customer who was confused with the wording. I agree that perhaps we should make it more clear in the documentation

  5. Bryan Turner Account Deactivated

    I've revised the wording on the Marketplace listing to hopefully make it more clear that unapproval on target branch changes is limited to when the branch name changes, not when new commits are pushed to the target branch.

  6. Log in to comment