Issues

Issue #7042 new

Remove approval of pull request when new commits are made after approval (BB-6784)

Gray Gilmore
created an issue

We really like the approval feature on Pull Requests, but we hit a situation today where I had approved something, then changes were made (new commits) but my approval remained, despite me having never seen the new changes.

We think it would be great if all approvals were reset when new commits are made to a pull request, otherwise we'll simply not use the feature.

Thanks!

Comments (22)

  1. Paul Melekhov

    I really can't understand how approvals functionality was implemented without this obvious thing. Without this the approval function is unusable because it misleading. And I believe this is not rocket science issue. This issue (which waiting more than 1 year) can be made of 1-3 lines of code.

  2. Alexander Ashitkin

    Please consider the case #10309 as well - "successful builds" flag should follow the same semantic. If pull request branch was moved to a new commit - 'successful build' flag should be inherited from that build or cleared if no builds data were recorded

  3. Matt Broadstone

    This is a critical feature for our current workflow. As a maintainer of a number of projects hosted on bitbucket, it's incredibly difficult for me to keep track of whether participants have accurately reviewed the pull request. There is definitely a clear mental disconnect presented with the feature as currently implemented: if I approve a pull request, I have indicated that I have read it/checked it/run the code/etc and agree that it does what it says it does, and agree with the author of the patch(es). If I approve something, and the underlaying code changes, I am now placed in an awkward situation where I've given my go-ahead but have no idea what I've signed off on anymore (the newly updated commits could COMPLETELY change the PR in the worst case).

    As the maintainer in many of these cases I become more of an "arbiter" (as I only have commit access to the master repo), and so I need to depend on my team to properly review the incoming code before merging in. The way the reviews currently work, I need to check in with the team members almost constantly to make sure that they have checked every mutation of the PR as things are being fixed based on points brought up in the discussion.

    It would ideal if two features could be added (or at least one or the other):

    • a rest api call to clear all approvals on a given pull request. If this could be done (as the administrator of a project), I could at least write a bot to watch for newly added commits/changes to existing commits on a PR and do the work myself

    • a checkbox in the project configuration to "opt-in" for automatically clearing approvals whenever any change has been made on the "source side" of a PR

  4. Alan Fregtman

    +1 New commits could introduce bugs so the pull should go back to an unapproved state.

    If you don't want to change existing behaviour, at least make it an option in the repo settings.

  5. Daniel Rech

    This is not even a feature request, it is a loophole fix.

    I would advise everyone voting for this to invite your colleagues to vote as well. As it stands, it is still too low in the voting ranking of issues.

  6. Log in to comment