To be Reviewed By: 2020.11.25

Authors: Dan Smith

Status: Draft | Discussion | Active | Dropped | Superseded

Superseded by: N/A

Related: N/A

Problem

Geode contributors don't always know who are the right people to tag on a review. Geode committers also tend to review and approve pull requests based on different criteria - for example to see if the documentation is well written, follows style-guides, or if the code meets minimum standards for including tests, etc. We want to ensure that all PRs are routed to subject matter experts that can review the substantive changes of the PR. We also want to ensure that contributors and committers can identify that a PR has been reviewed by someone with expertise in the correct area.

Solution

Use a github CODEOWNERS file to indicate owners for different areas of the code. None of these areas should have a single owner, and owners can and should rotate over time. It is possible that some areas of the project will have no owner. 

Being an owner means that a committer will automatically be added as a reviewer for a PR if that PR touches the given area.

In addition, the ASF provides the ability to require owner review using the asf.yml file. We will turn on the require_code_owner_reviews flag in .asf.yml, so that an owner's review will be required to merge. For changes without an owner, GitHub allows merging with the required review-count being met (currently: 1).

Nomination of owners

To initially populate the CODEOWNERS file we will have a nomination period where any committer can nominate themselves or another committer to be owners of different areas of code by making PRs against the CODEOWNERS file on the feature/introduce-codeowners branch. After that period we will agree on the list of owners and merge the CODEOWNERS file to develop.

Future changes to the CODEOWNERS should be done with a PR against the CODEOWNERS file and a email to the dev list.

If no one signs up to be the owner of an area of code that area will  be left unowned and not require any owner approval.

On committing the CODEOWNERS file, we will apply the .asf.yml file to the master branch of apache/geode, to enable owner-review requirements.

Suggested Breakdown of Owners:


 geode repository CODEOWNERS

The geode repository is too big to have a single set of owners. Here is a proposed breakdown of the geode repository into functional areas. https://github.com/apache/geode/blob/feature/introduce-codeowners/CODEOWNERS.

Other repositories

These repositories are smaller and only need at most a single set of code owners for each repository.

  • geode-native
  • geode-examples
  • geode-benchmarks
  • geode-site
  • geode-kafka-connector
  • geode-dotnet-core-client

Prior Art

What would be the alternatives to the proposed solution? What would happen if we don’t solve the problem? Why should this proposal be preferred?

FAQ

Answers to questions you’ve commonly been asked after requesting comments for this proposal.

Errata

What are minor adjustments that had to be made to the proposal since it was approved?

  • No labels

2 Comments

  1. I guess this RFC can be moved to "Implemented", am I right?