• Our product teams collect and evaluate feedback from a number of different sources. To learn more about how we use customer feedback in the planning process, check out our new feature policy.

      Update – This feature has been launched to allow reviewers to mark a pull request as "Requests Changes." This also allows customers to set up merge checks that block the merge of PRs until this status has been cleared by reviewers. As part of this work, we also moved the reviewers over into a sidebar so that it is easier to see which reviewers have indicated particular statuses. See screenshot below of attached functionality.

            [BCLOUD-13021] "Needs work" reviewer status

            We've released a few follow-on features related to this:

            • We've updated the dashboard at https://bitbucket.org/dashboard/overview to include a "Needs my review" filter which will display pull requests that you don't have a current status (either "Approved" or "Requested Changes") set on.
            • We've added a new branch permission (configurable by repo admins) that allows you to specify if you'd like for all "Requested Changed" statuses on a PR to be reset when a change is pushed to the source branch for each PR.

            If you have any issues or additional comments feel free to leave a note here and I'll take a look or reach out to our support team.

            Brandon Reppert (Inactive) added a comment - We've released a few follow-on features related to this: We've updated the dashboard at https://bitbucket.org/dashboard/overview  to include a "Needs my review" filter which will display pull requests that you don't have a current status (either "Approved" or "Requested Changes") set on. We've added a new branch permission (configurable by repo admins) that allows you to specify if you'd like for all "Requested Changed" statuses on a PR to be reset when a change is pushed to the source branch for each PR. If you have any issues or additional comments feel free to leave a note here and I'll take a look or reach out to our support team.

            d5949f2493ca I'll give updates for the follow-on features here rather than use a separate issue tracker as I believe that will give better visibility.

            Both dashboard filtering and configurable admin branch permissions for automatically resetting changes requested statuses on source branch changes are close at this point and I expect it will be no longer than the next few weeks that we'll be releasing these pieces of functionality.

            Brandon Reppert (Inactive) added a comment - d5949f2493ca  I'll give updates for the follow-on features here rather than use a separate issue tracker as I believe that will give better visibility. Both dashboard filtering and configurable admin branch permissions for automatically resetting changes requested statuses on source branch changes are close at this point and I expect it will be no longer than the next few weeks that we'll be releasing these pieces of functionality.

            Thanks @Brandon, that sounds good. I like @nichmj01's suggestion of the allowing the author the ability to indicate they have fixed the issues manually even better though I think. I guess in some ways this feature overlaps with the "tasks" functionality which can be used for more fine-grained change requests.

            Thanks for all your hard work on this feature.

             

            Matt Holgate added a comment - Thanks @Brandon, that sounds good. I like @nichmj01's suggestion of the allowing the author the ability to indicate they have fixed the issues manually even better though I think. I guess in some ways this feature overlaps with the "tasks" functionality which can be used for more fine-grained change requests. Thanks for all your hard work on this feature.  

            ojnick added a comment - - edited

            I also think it would be nice for the PR author to be able to indicate that they think they have addressed the Requested Changes. That will allow the PR Reviewers the ability to see if the code is ready to for the next round of reviews. This could be done by giving the PR author the ability to remove the Request Changes status or maybe there is another status that could be added like a Changes Addressed: Request Re-Review (or something shorter).

            ojnick added a comment - - edited I also think it would be nice for the PR author to be able to indicate that they think they have addressed the Requested Changes. That will allow the PR Reviewers the ability to see if the code is ready to for the next round of reviews. This could be done by giving the PR author the ability to remove the Request Changes status or maybe there is another status that could be added like a Changes Addressed: Request Re-Review (or something shorter).

            @Brandon - That sounds perfect! Is there a separate issue to watch for the "Opt-In to Automatically Clear Pull Request "Requests Changes" Status on New Commits" changes, or will that be completed under this issue?

            Tyler Hadidon added a comment - @Brandon - That sounds perfect! Is there a separate issue to watch for the "Opt-In to Automatically Clear Pull Request "Requests Changes" Status on New Commits" changes, or will that be completed under this issue?

            We're currently working on a feature that will make it configurable at the branch level by repository admins to set if changes will trigger an automatic removal of all "Requests Changes" states.

            We had a similar discussion internally and decided that the workflow we'd encourage would be to have the status stick so that the reviewer can verify that the change did in fact remove their concerns, but that there are teams where it's more desirable to clear the statuses automatically and thus we'd make an opt-in configuration for clearing the statuses.

            Brandon Reppert (Inactive) added a comment - We're currently working on a feature that will make it configurable at the branch level by repository admins to set if changes will trigger an automatic removal of all "Requests Changes" states. We had a similar discussion internally and decided that the workflow we'd encourage would be to have the status stick so that the reviewer can verify that the change did in fact remove their concerns, but that there are teams where it's more desirable to clear the statuses automatically and thus we'd make an opt-in configuration for clearing the statuses.

            alexey added a comment -

            I agree that automatic removal of "Needs work" state may not be desirable, but I have to agree with @Matt Holgate that it should be possible for PR author to reset all "Needs work" states and send an automatic email to reviewers who requested it. We are currently using Jira with "Code review" status to track who is currently responsible for the review (reassigned back and force between the author and the main reviewer) and I hoped that "Needs work" status could make it much easier but it doesn't, unfortunately.

            alexey added a comment - I agree that automatic removal of "Needs work" state may not be desirable, but I have to agree with @Matt Holgate that it should be possible for PR author to reset all "Needs work" states and send an automatic email to reviewers who requested it. We are currently using Jira with "Code review" status to track who is currently responsible for the review (reassigned back and force between the author and the main reviewer) and I hoped that "Needs work" status could make it much easier but it doesn't, unfortunately.

            @Mikhail KopylovI don't think auto resetting the "needs work" is a good idea as it implies any commit is the resolve to the reason the "needs work" was added.

            It should be up to the reviewer to see the new commits and if it resolves the reason it needed work then remove the needs work. It could even be that the new commits need work, for different reasons.

            Greg Sanderson added a comment - @Mikhail KopylovI don't think auto resetting the "needs work" is a good idea as it implies any commit is the resolve to the reason the "needs work" was added. It should be up to the reviewer to see the new commits and if it resolves the reason it needed work then remove the needs work. It could even be that the new commits need work, for different reasons.

            My 2 cents about that.

            First, thank you very much, the feature is very useful.

            I would be nice to reset "Needs Work" button once new commits have been pushed. Is that possible?

            Mikhail Kopylov added a comment - My 2 cents about that. First, thank you very much, the feature is very useful. I would be nice to reset "Needs Work" button once new commits have been pushed. Is that possible?

            Is there a way for the original author (or someone else) to remove someone's "Changes requested" flag? I can see this would be a problem if someone requested changes then went on vacation for 2 weeks, as it would block merging.

            Would it make sense to make the "Reset approvals on change" option to also reset the changes requested?

            Matt Holgate added a comment - Is there a way for the original author (or someone else) to remove someone's "Changes requested" flag? I can see this would be a problem if someone requested changes then went on vacation for 2 weeks, as it would block merging. Would it make sense to make the "Reset approvals on change" option to also reset the changes requested?

              mgong@atlassian.com Ming Gong (Inactive)
              daf4ee7b0267 ssummerer
              Votes:
              552 Vote for this issue
              Watchers:
              232 Start watching this issue

                Created:
                Updated:
                Resolved: