# Remove CODEOWNERS

<!--
Part of the Carbon Language project, under the Apache License v2.0 with LLVM
Exceptions. See /LICENSE for license information.
SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-->

[Pull request](https://github.com/carbon-language/carbon-lang/pull/1367)

<!-- toc -->

## Table of contents

-   [Problem](#problem)
-   [Background](#background)
    -   [Current CODEOWNERS](#current-codeowners)
    -   [CODEOWNERS issues](#codeowners-issues)
-   [Proposal](#proposal)
-   [Details](#details)
    -   [Group access controls](#group-access-controls)
    -   [rm CODEOWNERS](#rm-codeowners)
    -   [Auto-assignment](#auto-assignment)
    -   [Stacked PRs](#stacked-prs)
-   [Rationale](#rationale)
-   [Alternatives considered](#alternatives-considered)
    -   [Fine-grained CODEOWNERS](#fine-grained-codeowners)
    -   [Broaden CODEOWNERS](#broaden-codeowners)

<!-- tocstop -->

## Problem

We want to restrict who can merge files, in particular to ensure reviews are
done.

Currently we use CODEOWNERS for this purpose. However, CODEOWNERS can lead to
review bottlenecks, creating possibly unnecessary hindrances to development, as
discussed in the background.

## Background

### Current CODEOWNERS

We currently use, in brief:

```CODEOWNERS
*                   @carbon-language/implementation-team
/*                  @carbon-language/implementation-team
/*.md               @carbon-language/carbon-leads @carbon-language/implementation-team
/docs/              @carbon-language/carbon-leads @carbon-language/implementation-team
/proposals/         @carbon-language/carbon-leads
/LICENSE            @carbon-language/carbon-leads
CODEOWNERS          @carbon-language/carbon-leads
```

### CODEOWNERS issues

[Issue #413: Consider removing CODEOWNERS -- at least for now](https://github.com/carbon-language/carbon-lang/issues/413)
was used to discuss this issue. This can lead to a few issues:

-   CODEOWNERS defaults to assignment to the group, which can lead to a "tragedy
    of the commons" situation where everybody expects someone else to review.
    -   In assigning to the group, CODEOWNERS also makes for noisy PR
        notifications.
    -   Note both of these issues can be addressed with
        [GitHub review settings](https://docs.github.com/en/organizations/organizing-members-into-teams/managing-code-review-settings-for-your-team).
-   CODEOWNERS doesn't accurately handle the difference between a "major" change
    and a "minor" change.
    -   For example, a new proposal should only be approved by carbon-leads,
        while a typo fix to a proposal would be fine for anyone to approve.
    -   Decisions to make minor proposal edits more common, as in
        [Issue #1009: Should doc edits maintain proposal links?](https://github.com/carbon-language/carbon-lang/issues/1009),
        exacerbate this issue.

## Proposal

Remove CODEOWNERS, and instead use commit access to gate PRs.

## Details

### Group access controls

-   Base: read (unchanged)
    -   Should be the standard public permission.
-   Contributors: "contributor" (previously write)
    -   The "Contributors" group should be easy to get access to. On top of
        read, this should include the ability to add/remove labels and projects
        (may require some work to get right).
    -   Versus "triage", "triage" seems to have powers including deleting
        conversations, etc, that may not make sense for this group.
-   Implementation team: write (unchanged)
    -   For people expected to be reviewing commits.
-   Moderators: triage
    -   Includes
        [separate GitHub moderation permissions](https://docs.github.com/en/communities/moderating-comments-and-conversations).
-   Leads: maintain (unchanged)
-   Admin team: admin (unchanged)

For people who have access, settings are at:

-   [Organization repository roles](https://github.com/organizations/carbon-language/settings/roles)
-   [Organization moderators](https://github.com/organizations/carbon-language/settings/moderators)
-   [Repository collaborators and teams](https://github.com/carbon-language/carbon-lang/settings/access)

### rm CODEOWNERS

This should only be done after group access controls are updated. Once done,
group access controls are the last word on who can commit PRs.

### Auto-assignment

This PR [introduces auto-assignment](/.github/workflows/auto_assign_prs.yaml) in
order to ensure PRs aren't lost. It provides categories of assignment, and a
fallback for other PRs that don't have explicit assignment.

### Stacked PRs

Advice for stacked PRs is no longer "just ask an admin"; it is "commit access
required". This is because removing CODEOWNERS removes boundary between merge
access _to a feature branch_ and merge access _to trunk_.

## Rationale

-   [Community and culture](/docs/project/goals.md#community-and-culture)
    -   The intent is to make it easier to get PRs reviewed and reduce
        bottlenecks.

## Alternatives considered

### Fine-grained CODEOWNERS

We could keep the [current fine-grained CODEOWNERS](#current-codeowners). In
this setup, we would try to have specific directories owned by specific teams.

We've ended up on this path so far so that we have some mechanical enforcement
of who's reviewing. We may need to change up the groups a little, but a
reasonable long-term expectation would be something like a docs team, an
explorer team, a toolchain team, etc, with each owning their own files. For
example, /docs is owned only by the docs team, and there are people in the docs
team who aren't in other teams (and the other way around).

As previously mentioned, if we were using this option, we may want to change
[GitHub review settings](https://docs.github.com/en/organizations/organizing-members-into-teams/managing-code-review-settings-for-your-team)
in order to fix email/assignment.

Advantages:

-   Relies on built-in GitHub CODEOWNERS featureset, intended for this purpose.
    -   Can use built-in features for review assignment.
-   Works with current stacked PR advice.

Disadvantages:

-   Retains team-specific bottlenecks: if none of a directory's owners are
    available, other contributors cannot step in.
    -   This could also be considered to be an advantage by some.
-   GitHub's built-in features for review assignment are not as feature-rich as
    some custom tooling.

It's mainly because of the relative inflexibility of CODEOWNERS that we aren't
doing this.

### Broaden CODEOWNERS

We could broaden the CODEOWNERS to something basic, like a single group owning
everything. Reviews in a particular area become best practice, not mechanically
enforced.

Advantages:

-   Simple to understand.
    -   Avoids custom tooling.

Disadvantages:

-   We expect multiple subprojects within the carbon-lang repository, and this
    approach would make it difficult to determine when to review a PR.
    -   A large group means GitHub's built-in auto-assignment would assign to
        arbitrary CODEOWNERS.
    -   The [notification issues](#codeowners-issues) couldn't be addressed.

This option is likely worse for our purposes than
[fine-grained CODEOWNERS](#fine-grained-codeowners).
