Create
cancel
Showing results for 
Search instead for 
Did you mean: 
Sign up Log in

Is it possible for pull-request author to be also a reviewer ?

Laurent Etiemble December 14, 2016

This question is in reference to Atlassian Documentation: Using pull requests in Bitbucket Server

I would like to know if a pull-request author can also be a pull-request reviewer. 

It seems that I cannot add myself as reviewer when I create a pull-request.

 

3 answers

10 votes
Laurent Etiemble December 14, 2016

Here is the use-case:

  • Several developers commit on a branch.
  • The project manager creates the pull-request; he is the "author" of the PR.
  • The project manager wants to be part of the review.
  • The project manager accept/refuse the PR based on the reviews.

 

 

0 votes
Roger Barnes
Atlassian Team
Atlassian Team members are employees working across the company in a wide variety of roles.
December 14, 2016

Is there a reason you'd find this useful, Laurent? We assume that when someone creates a pull request for others to review that they already approve of their own changes, else they would keep working on them before creating a pull request.

Derrick Slopey December 24, 2019

The PR author may add something to the pull request and wish for someone else to review it.  Excluding the PR author from the initial auto-reviewers (default reviewers) makes sense. Not allowing to manually add the PR author to the list of reviewers is unnecessarily limiting.

Like # people like this
Roger Barnes
Atlassian Team
Atlassian Team members are employees working across the company in a wide variety of roles.
January 1, 2020

In what way(s) do you find it limiting in practice, @Derrick Slopey ?

Laurent Etiemble January 2, 2020

The use-case we would like to address is where we have a long-lasting branch on which we merge small branch. The PR based on the long-lasting branch is created by a member of the Team and we would like to allow this member to contribute to the review.

For now, we managed to work around the limitation by changing the way we work.

Olaf Lenz January 14, 2020

Another use case could be if the author wants to allow the reviewers to do the review, but still wants to delay the merge e.g. for some tests to be successful before the PR should be merged.

I can think of a lot of reasons why an author might want to delay a PR from being integrated, but doesn't want to delay the review.

Like Steffen Opel _Utoolity_ likes this
Roger Barnes
Atlassian Team
Atlassian Team members are employees working across the company in a wide variety of roles.
January 14, 2020

@Laurent Etiemble , could you elaborate on what the limitation is in practice? The author can still review (ie comment), and their creation of the PR implies they approve of the changes. Is that the gap, perhaps, where they might not yet fully approve of the combined changes and lack the ability to indicate their disapproval for merge upon inspection?

 

@Olaf Lenz it sounds like you're describing a different need? I'm unsure how the PR author being a reviewer would help to delay the merge.

Laurent Etiemble January 14, 2020

@Roger Barnes I agree with you. If you are a PR author, you implicitly approve your changes. As I said before, we have revised the way we work to conform to the current review system: you cannot be both a PR author and a reviewer.

Olaf Lenz January 14, 2020

@Roger Barnes indeed, I now notice that I have left out important information: we have an integration team that will merge PRs as soon as all reviewers have approved. The developers are not allowed to do integrations into the master branch themselves.

For that use model, it is sometimes important for the creator of a PR to be able to delay the integration, even though he already wants to start the review of the PR. This can happen, for example, when the creator wants to wait for certain tests to have finished, or when he wants to do certain rework.

A simple way how to achieve that would be if the author could add himself as a reviewer. Then he could approve the PR in the moment that he wants the integration to proceed.

Of course, other solutions would also work, e.g.

  • a button for the author to release the PR
  • the author can add another reviewer "Integration Blocker" that he removes from the PR when he is done
  • ... 

Should I write another question for this use case?

Roger Barnes
Atlassian Team
Atlassian Team members are employees working across the company in a wide variety of roles.
January 16, 2020

Thanks Olaf, I think one of the other mechanisms you mention would make the most sense for that use case. For the idea of putting a PR "on hold" , there is an open suggestion that is gathering interest: https://jira.atlassian.com/browse/BSERV-9357

jbarcenilla May 19, 2020

I'm also having problems with this.

We have to support multiple version of our products and we have branches like:

  • release/1.0
  • release/2.0

We have enabled these checks:

  • Check for at least 1 approval
  • Check the last commit for at least 1 successful build and no failed builds

In general everything works fine. People can create hotfix branches, open a pull request and merge it following the configured workflow. 

At some point I want to merge release/1.0 into release/2.0 to bring all the hotfixes into the next version so I open a PR. I'm not the author and I don't need to review the code (it was reviewed in its own PR). 

I could do a manual merge but I want to run the pipelines to be sure the tests pass.

I could ask a college to artificially mark the PR as reviewed but I automated the sync between releases using pipelines that open and merge the PR automatically. Everything works until I enable the approval check.

The PR is not only a review system, it is also part of the CI/CD process and checks the builds. I think It wouldn't be again the purpose of the PR allow a specific user to skip the check or self-review it own PR. 

Roger Barnes
Atlassian Team
Atlassian Team members are employees working across the company in a wide variety of roles.
May 19, 2020

@jbarcenilla , thanks for sharing that example. I think you've the right approach overall, but I think the the better solution to your problem would be if you could customise the merge check (approval) requirements based on the branch types. Then you could not require an approver for release chain merges. Alas that is not currently an option.

Have you considered enabling the branch model with cascading merges such that all changes going to release/1.0 automatically flow to newer versions? Admittedly, that would change your workflow in that you would only find out about broken builds after the fact. It would come down to how much you value doing it that particular way and how it impacts your release processes.

jbarcenilla May 21, 2020

Thanks for your comments @Roger Barnes 

I'm new in Bitbucket but I think the merge checks can only be defined base on the destination branch. I want to require an approver when someone open a PR hotfix/XXX => release/XXX but i don't want it for a PR release/XXX => release/YYY.

About the cascading merges, if you mean this feature https://confluence.atlassian.com/bitbucketserver/automatic-branch-merging-776639993.html , I'm using Bitbucket Cloud so it is not available.

I've implemented my own Automatic branch merging using pipelines and the Bitbucket API. I'm using two different users so one open the PR and other approve it. It is not the most elegant way but it works for us until we have another option. I'll share the process in  other post when I have time.

Ashton Batty June 9, 2020

We follow a forking model, and I have set Minimum Approvals to 1, and all updates to the master repo for each application must be via a pull request.

My use case is that sometimes I need to create a pull request on someone else's change (eg. that person has left my team, or is away sick/on leave). That person may have changes that are complete and ready, and I might be the appropriate person to review and merge them, but if they are unavailable I need to get another lead (or person with appropriate permissions) to create the pull request (inefficient and mildly disruptive), or I need to temporarily disable the Minimum Approvals merge check for the repo, (which I really prefer not to do, I want a consistent record that I have approved the PR).

Like # people like this
Tom De Greyt June 23, 2021

@Roger Barnes 
Assuming the author's approval over the changes of his fellow commit authors implies that they were using some other code review tool among themselves before moving it to a Pull Request. i.e. To some teams the Pull request may just be the start of a finalization process, where they review each others contributions as well.
It's also odd to exclude the author of the Pull Request and not any other commit authors if you are making the assumption that the commit authors agreed their work was ready to be merged.
Bottom line, it's an assumption that might often be true in practice, but the strict restriction adds no value.

Max DeCurtins June 25, 2021

@Roger Barnes We are using Bitbucket Cloud Premium and I have enabled "1+ approval from default reviewers" as a merge check. I am the only default reviewer, and I also author the majority of the PRs, but I would prefer to be a default reviewer for PRs authored by other members of my team. As far as I can tell, this merge check can't be enabled on ANY branch if a default reviewer is also a PR author. The fact that I cannot add myself as a reviewer to a PR that I authored that will then be blocked from merging because I'm not a reviewer on the PR is, to put it mildly, absurd.

 

Edit: to be sure, one could have more than one default reviewer for a repository, but not to have accounted for the scenario of one default reviewer seems like a strange decision.

0 votes
Jobin Kuruvilla [Adaptavist]
Rising Star
Rising Star
Rising Stars are recognized for providing high-quality answers to other users. Rising Stars receive a certificate of achievement and are on the path to becoming Community Leaders.
December 14, 2016

You cannot. That beats the purpose of a review smile

Alexey_Efimov
Rising Star
Rising Star
Rising Stars are recognized for providing high-quality answers to other users. Rising Stars receive a certificate of achievement and are on the path to becoming Community Leaders.
December 14, 2016

Maybe it be usefull for dissociative identity disorder.

Suggest an answer

Log in or Sign up to answer
TAGS
AUG Leaders

Atlassian Community Events