Support for multiple CODEOWNERS sections
Problem to solve
The CODEOWNERS file is powerful because it allows rules to be dynamically applied by path, but it is limited by the fact that:
- only one rule can match each path
- all rules are enforced or no rules are enforced
If multiple teams (e.g. security, UX, frontend, backend) want to use code owners then they have to carefully negotiate which paths ping which teams. It is almost impossible to get desired results because each team will want to manage their approval interests independently without worrying about which rules take priority.
More problematically, all code owner rules must be enforced or none. This means that if there are important compliance rules that must be enforced, every rule must be enforced. This means another team that simply wants to be notified and added as optional approvers of relevant merge requests must forgo using code owners or become a required approver.
Further details
For example:
- the compliance team needs to make sure all changes to the
app/payments
code is reviewed by Jane - there are multiple development teams that work on the payment code, and want to automatically assign code reviews to the correct teams,
app/payments/checkout
,app/payments/renewal
andapp/payments/cancellation
With CODEOWNERS this isn't possible since only one rule can match - the most specific rule (last matching rule in the file)
Proposal
Allow multiple section in the CODEOWNERS
file, and the most specific rule from each section wins. That way each team can have their own file and not get in the way of each other.
- there is an implicit group with the name
CODEOWNERS
for rules not in a named section. Since it's implicit, it will not be shown on the MR widget as a named group. Members of this group will continue to show up next to the path as they do today - if there are multiple sections with the same name, including the implied section, these are treated as the same section in file order (since the file is parsed to to bottom)
- section names are case insensitive (e.g two sections
Database
anddatabase
will be parsed as the same section) - section name shown in the interface is taken from the first section name (e.g. if
Database
is beforedatabase
, thenDatabase
will be shown instead ofdatabase
) - the blob page (example) will continue to show specific users, not section. This may change in the future.
# rules before a defined section are implicitly treated as part of the group CODEOWNERS
README.md @example-user
[Documentation]
README.md @gl-docs
ee/docs @gl-docs
docs @gl-docs
[Database]
README.md @gl-database
model/db @gl-database
If someone changes the README today on the database group would be added as a reviewer. What we want is:
- example user
- docs
- database
to all be added as approvers in distinct sections so that all three can be required.
Mockup merge request widget | Mockup protected branches (follow-up #214609 (closed)) |
---|---|
If multiple sections are specified for the same path, they will be shown with the section name in the MR widget
Availability & Testing
- We should ensure that performance is acceptable when there is a large number (up to reasonable limits) of either rules, groups, files, or directories, or a large number of all of those.
- There are existing end-to-end tests that should be updated to include multiple CODEOWNERS sections to provide additional coverage: gitlab-org/quality/testcases#765
-
run package-and-qa
before changes are merged to confirm that existing end-to-end tests still pass.
Customers
- https://gitlab.my.salesforce.com/00161000004bZPD
- https://gitlab.my.salesforce.com/00161000017upDb
- https://gitlab.my.salesforce.com/0016100001F4x1EAAR
- https://gitlab.my.salesforce.com/0016100000fdr2y
- https://gitlab.my.salesforce.com/00161000006hNmo
Links / references
- A use-case where we would like to use this for
gitlab/gitlab