Scope merge request approval rules to protected branches
Problem to solve
Protected branches make it possible to control who may merge to a specific branch, but merge request approvals are a blunt because they apply to all branches equally. Merging to master
should have the strictest approval requirements, while merging to a feature branch requires none, and merging to a release branch requires different approval requirements.
Further details
Use case from customer:
Per branch permissions is a great feature and I look forward to it! It does help address some of my problems, but, as you said, not the merge request approvals part. I guess my biggest problem with merge approvals is that it applies to all merge requests for all branches of the project. I really only want to require approvals for our dev branch because that is where we’re having continual problems with broken code being checked in. Our master and patch branches are already locked down to a small handful of people with push/merge permissions so requiring approvals there doesn’t make sense for my teams. On the page I linked below (Drew: blog post linked above) I did see others asking for per-branch approvals, but not sure whether there is wide-spread demand for something like this. I realize that all feature requests can’t be done; some don’t make sense and there isn’t time for others. Hopefully it will be discussed again on your side if it hasn’t been already.
Proposal
As a maintainer configuring merge request approval rules, for each approval rule, I will be able to select which protected branch rules it should apply to.
Project settings | Add/edit approval rule modal | Dropdown open |
---|---|---|
Help text: Apply this approval rule to any branch or a specific protected branch.
|
It should be possible to select (single selection only, not multi-select):
- Any branch (default, current behaviour)
- One of the existing protected branch rules explicitly
Permissions and Security
Permissions for creating, editing and removing Approval Rules should be unchanged (Maintainers and above, existing behavior).
For existing merge requests, changes to the Project Approval Rules should be reflected in open merge requests, except where overridden (existing behavior).
Documentation
Testing
What does success look like, and how can we measure that?
This change is about making sure the correct people can assigned to a merge request as approvers automatically without manual overrides. This can be measure directly by the number of custom/modified merge request approval rules created - the rate of increase should decrease over time.
Furthermore, by allowing more specific rules, this should prevent required approval rules being added to merge requests between feature branches for example. This would mean more merge requests with zero approval rules. For merge requests that do still have approval rules, conversely the number of required approval rules per merge request should increase.
What is the type of buyer?
This feature will increase efficiency for all teams using Multiple Approval Rules ("GitLab Premium"), but will be of less significant benefit if only one Approval Rule can be configured ("GitLab Starter").
This feature will be very important to buyers with Compliance and Governance concerns who want to make sure that specific approvals take place at specific points in the workflow. (GitLab Ultimate)
Solution validation
Feedback collected in: https://gitlab.com/gitlab-org/create-stage/issues/12606
-
2019-10-11 https://gitlab.my.salesforce.com/00161000004zr1Y -
2019-11-13 https://gitlab.my.salesforce.com/00161000006fkPe -
https://gitlab.my.salesforce.com/00161000010fYg0 -
https://gitlab.my.salesforce.com/0016100001HGPYw -
https://gitlab.my.salesforce.com/0016100001EogBS
Links / references
- Feedback as comments: https://about.gitlab.com/2015/07/29/feature-highlight-merge-request-approvals/
- Zendesk: https://gitlab.zendesk.com/agent/tickets/19387
- Related to https://gitlab.com/gitlab-org/gitlab-ee/issues/179
Feature Flag
scoped_approval_rules
feature flag has been added in !22673 (merged). It's enabled by default but should be removed once this is verified both BE and FE.