Skip to content

Commit

Permalink
Maintainers policy (#1832)
Browse files Browse the repository at this point in the history
Fixes #516. 
* Small changes in "Code Reviews" section.
* Add new "Roles" section.
  • Loading branch information
mishmosh authored Feb 9, 2019
1 parent b4eb252 commit 4e75ee9
Showing 1 changed file with 77 additions and 4 deletions.
81 changes: 77 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ Filecoin, including go-filecoin and all related modules, follows the
* [Gotchas](#gotchas)
* [The Spec](#the-spec)
* [What is the bar for inclusion in master?](#what-is-the-bar-for-inclusion-in-master)
* [Roles](#roles)


## How can I contribute?
Expand All @@ -46,7 +47,7 @@ Check out the [Go-Filecoin code overview](CODEWALK.md) for a brief tour of the c

### Design Before Code
- Write down design intent before writing code, and subject it to constructive feedback.
- Major changes should have a [Design Doc](designdocs.md).
- Major changes should have a [Design Doc](https://github.com/filecoin-project/designdocs).
- For minor changes, file an issue for your change if it doesn't yet have one, and outline your implementation plan on the issue.

### Pull Requests
Expand All @@ -58,17 +59,22 @@ Check out the [Go-Filecoin code overview](CODEWALK.md) for a brief tour of the c

### Code Reviews

`go-filecoin` requires 2 approvals for all PRs, with at least one of {@phritz, @acruikshank, @whyrusleeping}. If your PR hasn't been reviewed in 3 days, pinging reviewers via Github or community chat is also welcome and encouraged.
`go-filecoin` requires 2 approvals for all PRs. We use the following process, which aims to merge code quickly and efficiently while avoiding both accidental and malicious introduction of bugs, unintended consequences, or vulnerabilities.

The first review is done by a committer familiar with that area of the codebase. Once they deem it ready for merge, they will assign a maintainer for the second review. Once the PR has 2 Approvals and blocking comments have been addressed, the committer rebases and merges the PR.

If your PR hasn't been reviewed in 3 days, pinging reviewers via Github or community chat is also welcome and encouraged.

We use the following conventions for code reviews:

- "Approve" means approval. If there are comments and approval, it is expected that you'll address the comments before merging. Ask for clarification if you're not sure.
- *Example: reviewer points out an off by one error in a blocking comment, but Approves the PR. Reviewee must fix the error, but the reviewer trusts you to do that.*
- *Example: reviewer points out an off by one error in a blocking comment, but Approves the PR. Reviewee must fix the error, but PR can progress to maintainer review. Committer confirms this before merge.*
- "Request Changes" means you don't have approval, and the reviewer wants another look.
- *Example: the whole design of an abstraction is wrong and reviewer wants to see it reworked.*

- [There is a proposal in progress to invert BLOCKING, to match most contributors' expectations.] By default, code review comments are advisory: the reviewee should consider them but doesn't _have_ to respond or address them. Comments that start with "BLOCKING" must be addressed and responded to. If a reviewer makes a blocking comment but does not block merging (by marking the review "Add Comments" or "Approve") then the reviewee can merge if the issue is addressed.
- By default, code review comments are advisory: the reviewee should consider them but doesn't _have_ to respond or address them. Comments that start with "BLOCKING" must be addressed and responded to. If a reviewer makes a blocking comment but does not block merging (by marking the review "Add Comments" or "Approve") then the reviewee can merge if the issue is addressed.

In rare cases, a maintainter may request approval from all maintainers for a wide-reaching PR.

#### Code Reviewer Responsibilities:

Expand Down Expand Up @@ -244,3 +250,70 @@ Presently (Q1'19) the minimum bar is:
* Lint (`go run ./build/*.go lint`).
* Must match a subset of the spec.
* Documentation is up to date.

### Roles

There are four main roles for people participating in `go-filecoin`. Each has a specific set of abilities and responsibilities: Contributors, Collaborators, Committers, and Maintainers.

#### Contributors

Anyone is welcome. If you have created an issue, commented on an issue or discussion forum thread, or submitted a PR, you are a contributor.

Responsibilities:

* Participate in the project, following the [Code of Conduct](https://github.com/filecoin-project/community/blob/master/CODE_OF_CONDUCT.md).

Abilities:

* Open issues and PRs
* Comment on issues and PRs

#### Collaborators

A **Collaborator** is someone who demonstrates helpful contributions to the project.

Responsibilities:

* Make helpful contributions via issues, PRs, and other venues

Abilities:
* Write to the repo (but cannot merge to master)
* Manage issues

#### Committers

A **Committer** is someone with a broad understanding of the codebase and the judgment and humility to call on others' expertise when needed. They have a consistent track record of quality contributions, regular participation, and enabling others.

Responsibilities:

* Review PRs and guide work to ready-to-merge
* With maintainer approval, rebase and merge contributor PRs
* Issue triage and other project stewardship

Abilities:

* Manage issues
* Merge PRs

#### Maintainers

A **Maintainer** is someone:

1. **who is invested in and broadly familiar with the project**, as demonstrated by a history of significant technical-, process-, and/or project-level contributions;
2. **who deeply understands the system**, especially knowing when and who to defer to as a reviewer, and with an eye towards unintended consequences;
3. **who is actively engaged in project progress and stewardship** by enabling others through project-wide planning, code reviews, design feedback, etc.; and
4. **who is a model of trustworthiness, technical judgement, civility, and helpfulness**.

Responsibilities:

* Review: Timely, friendly review of PRs and design docs to ensure high-quality code and grow knowledge of committers and frequent contributors
* Planning and Improvements: Participate meaningfully in technical and process-related improvements at the project level
* Make significant, direct technical contributions
* Backstop for hard problems and general project stewardship (TODO: improve wording)

Abilities:

* Manage issues
* Merge PRs

**Becoming a committer or maintainer:** Anyone can nominate someone for committership or maintainership by filing an issue {TODO: link to correct repo} pointing to evidence that the candidate (1) meets the definition and (2) is already performing the responsibilities decsribed in Roles. Existing maintainers must unanimously approve the new candidate. Removing a committer or maintainer requires either self-nomination, or confirmation by at least 66% of existing maintainers.

0 comments on commit 4e75ee9

Please sign in to comment.