two level code review using git pull request in BitBucket

softwareengineering.stackexchange https://softwareengineering.stackexchange.com/questions/356169

Question

The following procedure: We have Development branch and

  1. Developer create feature branch from Development
  2. Work and commit in this branch
  3. Rebase from Development
  4. Create pull request
  5. Code review
  6. Merge with Development

Problem that we should have 2 level code review, performing by different people. After first review approval, it goes for second review and merging only after this.

How to do this? Should we add additional branch between Feature and Development branches? Or some more simple way?

Was it helpful?

Solution

I do this quite frequently, for microservices other teams own, or for documentation changes to tech pubs. We have separate repos for individual microservices, with only a scrum team or two having merge permissions for each. I want my own team to review thoroughly before I waste the time of the owning team.

The easiest way I have found is to fork the repo into my private profile, then do all my work from a branch on my fork. I do a pull request against master on my fork first, then when that is approved, do a pull request against master on the "official" repo.

The reason I started doing it this way is our team is somewhat of a trusted innovator, so other teams would often just merge anything we sent them without questioning it. Doing the first pull request in a separate repo meant they couldn't merge it prematurely just by clicking a button. The permissions and visibility on both repos are easily controlled.

You don't have to do this with personal forks, you could create repos for longer-lived larger groups if that works for your use case. Perhaps one repo for everything from a certain contractor company, for example.

OTHER TIPS

I see two ways you could handle this.

  1. Set up a second pull request after the first one is successful, making sure not to complete the first PR and merge it in. Downside here is the possibility that the branch is accidentally merged.

  2. Add both people (or groups) to the pull request, and require them both to sign off before it can be completed. Once both have signed off, merge it in. The second person gets notified of the PR before the first has signed off, so they may be looking at a PR that is not "ready for them". If your PRs are usually rejected or require a lot of changes, that may be a problem, but otherwise shouldn't be.

The second option makes the most sense to me, as there is no chance of the branch getting accidentally merged before both PRs have been done.

Bitbucket has a policy feature which can enforce two-levels of review without any process changes:

The easiest policy is to enforce that a few people look at the new feature or bug fix before it's merged. For example, many teams decide that a pull request can only be merged if at least two developers have reviewed and approved the code. Your team may want to set an upper limit on the number of reviewers to prevent slowing down the progress too much but it's often useful to invite more reviewers than the minimum approval limit so that the progress on the review is not stalled by busy team members

This can be implemented by upgrading:

If your team has a Premium plan, repository admins can prevent pull requests that don't have a certain number of approvals from merging.

or by installing a plugin which validates commits:

Commit Policy Plugin checks the changes committed to Bitbucket Server repositories against a set of configurable rules (the commit policy). It does so by integrating the Commit Policy Plugin for Jira with Bitbucket Server. When the policy is not satisfied, the commit is rejected. Rejected commits can then be "fixed" and re-committed.

References

Licensed under: CC-BY-SA with attribution
scroll top