mirror of
https://github.com/carbon-language/carbon-lang.git
synced 2026-09-24 19:40:10 +01:00
Getting commit access (#4246)
Establish a process for getting commit access. We will:
- Grant access based on a developer's commit history.
- Someone with commit access should nominate, and a contributor may ask.
- A lead will approve nominations. Only one lead is needed.
- Remove commit access once someone is idle for 6 months.
- "Idle" means no significant project activity on any of GitHub,
Discord,
or in meetings.
- Access removed due to being idle will be restored on request.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
This commit is contained in:
Jon Ross-Perkins
co-authored by
Chandler Carruth
Chandler Carruth
parent
a24816a1f4
commit
8b0154ce85
@@ -0,0 +1,83 @@
|
||||
# Commit access
|
||||
|
||||
<!--
|
||||
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
|
||||
-->
|
||||
|
||||
<!-- toc -->
|
||||
|
||||
## Table of contents
|
||||
|
||||
- [Overview](#overview)
|
||||
- [Getting access](#getting-access)
|
||||
- [Removing access](#removing-access)
|
||||
|
||||
<!-- tocstop -->
|
||||
|
||||
## Overview
|
||||
|
||||
First and foremost, commit access is **not required** for contributing to
|
||||
Carbon! Anyone can send a pull request with a change to Carbon. Very few parts
|
||||
of our process require commit access, and most development activity is exactly
|
||||
the same for folks with or without. This is an important feature of our GitHub
|
||||
workflow for us, and one we plan to keep.
|
||||
|
||||
The main thing commit access allows is for developers to merge PRs into the main
|
||||
[carbon-lang repository](https://github.com/carbon-language/carbon-lang/). They
|
||||
still need review, and often a reviewer with commit access handles the merge. It
|
||||
also gives the ability to [approve and merge changes](code_review.md).
|
||||
|
||||
Getting commit access can help ease the development process; for contributors
|
||||
who don't feel comfortable doing reviews, it can still ease the review process
|
||||
for their own PRs. It allows authors to automatically get GitHub Action results
|
||||
and merge their own PRs, including after resolving conflicts or fixing trivial
|
||||
PR comments post-approval.
|
||||
|
||||
Developers with commit access are expected to make reasonable choices about when
|
||||
to approve or merge PRs for others. Generally, the change either needs to be
|
||||
trivially understood without context on the code in question (such as a cleanup
|
||||
or typo fix), or the developer should have some context on the code in question.
|
||||
If in doubt, feel free to review but leave approving or merging for someone
|
||||
else.
|
||||
|
||||
Similarly, developers with commit access are expected to make reasonable choices
|
||||
about what changes to make to their own PRs without asking for another round of
|
||||
review prior to merge, even after approval. And again, if in doubt, ask for
|
||||
review.
|
||||
|
||||
## Getting access
|
||||
|
||||
Contributors can request commit access based on their commit history. We want to
|
||||
make sure that developers with commit access are reasonably familiar with the
|
||||
style and structure of the codebase. Access will typically be granted when
|
||||
developers have contributed several PRs and are expecting to continue making
|
||||
more.
|
||||
|
||||
After a few non-trivial PRs are merged, contributors can ask a reviewer to
|
||||
nominate them for commit access if they plan to keep contributing. Reviewers can
|
||||
also directly suggest and nominate someone. Nominations need to be approved by
|
||||
at least one lead.
|
||||
|
||||
When approved, an invitation will be sent to join the
|
||||
[Commit access team](https://github.com/orgs/carbon-language/teams/commit-access)
|
||||
on GitHub. The invitation should cause a notification from GitHub, but it's also
|
||||
possible to go to the
|
||||
[invitation link](https://github.com/orgs/carbon-language/invitation) directly.
|
||||
Access is granted when the invitation is accepted.
|
||||
|
||||
## Removing access
|
||||
|
||||
We'll periodically remove commit access from contributors who have been idle for
|
||||
over 6 months. We'll use a combination of sources to determine what "idle"
|
||||
means, including whether a developer has been either sending or reviewing PRs.
|
||||
Developers can ask for commit access to be restored if they plan to start
|
||||
contributing again or come back from a break. Any relevant past contributions
|
||||
will still apply and allow it to be restored trivially.
|
||||
|
||||
For example, for `jonmeow`, GitHub searches (defaulting to PRs for convenience,
|
||||
but other types are included):
|
||||
|
||||
- [repository:carbon-language/carbon-lang author:jonmeow](https://github.com/search?q=repository%3Acarbon-language%2Fcarbon-lang+author%3Ajonmeow&type=pullrequests)
|
||||
- [repository:carbon-language/carbon-lang commenter:jonmeow](https://github.com/search?q=repository%3Acarbon-language%2Fcarbon-lang+commenter%3Ajonmeow&type=pullrequests)
|
||||
@@ -125,10 +125,7 @@ any of the above community groups.
|
||||
### GitHub commit access
|
||||
|
||||
Developers who can merge and approve changes on GitHub. See
|
||||
[commit access](https://github.com/carbon-language/carbon-lang/pull/4246) for
|
||||
information.
|
||||
|
||||
> TODO: link to commit_access.md when finalized.
|
||||
[commit access](commit_access.md) for information.
|
||||
|
||||
- GitHub teams:
|
||||
[Commit access](https://github.com/orgs/carbon-language/teams/commit-access)
|
||||
|
||||
@@ -0,0 +1,124 @@
|
||||
# Getting commit access
|
||||
|
||||
<!--
|
||||
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/4246)
|
||||
|
||||
<!-- toc -->
|
||||
|
||||
## Table of contents
|
||||
|
||||
- [Abstract](#abstract)
|
||||
- [Problem](#problem)
|
||||
- [Background](#background)
|
||||
- [Commit access](#commit-access)
|
||||
- [Related policies](#related-policies)
|
||||
- [Proposal](#proposal)
|
||||
- [Rationale](#rationale)
|
||||
- [Alternatives considered](#alternatives-considered)
|
||||
- [Longer idle times](#longer-idle-times)
|
||||
|
||||
<!-- tocstop -->
|
||||
|
||||
## Abstract
|
||||
|
||||
Establish a process for getting commit access. We will:
|
||||
|
||||
- Grant access based on a developer's commit history.
|
||||
- Someone with commit access should nominate, and a contributor may ask.
|
||||
- A lead will approve nominations. Only one lead is needed.
|
||||
- Remove commit access once someone is idle for 6 months.
|
||||
- "Idle" means no significant project activity on any of GitHub, Discord,
|
||||
or in meetings.
|
||||
- Access removed due to being idle will be restored on request.
|
||||
|
||||
## Problem
|
||||
|
||||
Right now, we have an undocumented process for getting commit access, and no
|
||||
real agreement for when to remove it. Commit access is important, so rather than
|
||||
just making a documentation edit, I'm submitting this as a proposal.
|
||||
|
||||
## Background
|
||||
|
||||
### Commit access
|
||||
|
||||
When we say "commit access", what we mean is the ability to push commits to the
|
||||
main `carbon-lang` repository, regardless of branch. Some details about the
|
||||
implications:
|
||||
|
||||
- Review and approvals would remain required for pushes to `trunk`.
|
||||
- This does grant access to push to other branches, although we will
|
||||
continue to encourage fork-based workflows.
|
||||
- Pushing PRs becomes easier with commit access.
|
||||
- Action workflows will automatically execute, instead of requiring a
|
||||
per-commit approval by someone with commit access.
|
||||
- Modifying PRs after review approval is possible.
|
||||
- This is both good (for small updates such as fixing typos and
|
||||
resolving conflicts) and bad (particularly for code security).
|
||||
- Communication delays can make it hard for an author to resolve conflicts
|
||||
and reviewer to approve and merge before more conflicts are introduced.
|
||||
- A possible solution to the conflict problem is to have the reviewer
|
||||
merge PRs more frequently, and regardless of any decision here, we
|
||||
may eventually need to adopt that approach.
|
||||
- Commit access functionally means the ability to approve and merge PRs from
|
||||
others.
|
||||
- As alternatives, we could use either a separate GitHub team for
|
||||
approvals or
|
||||
[CODEOWNERS](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners).
|
||||
We have avoided these so far because we're small, having backup
|
||||
approvers can be helpful, and mistakes are easy to undo.
|
||||
- This does not include the ability to push to other repositories. While there
|
||||
are a few in the `carbon-language` organization, only the `carbon-lang`
|
||||
repository is actively used.
|
||||
- People with commit access in effect have access to secrets, can make
|
||||
releases, and so on.
|
||||
|
||||
### Related policies
|
||||
|
||||
This proposal does not supersede other project policies, in particular:
|
||||
|
||||
- [CLA](/CONTRIBUTING.md#contributor-license-agreements-clas)
|
||||
- [Code of Conduct](/CODE_OF_CONDUCT.md)
|
||||
- [Review process](/docs/project/code_review.md)
|
||||
|
||||
## Proposal
|
||||
|
||||
The key things I think should be covered in this proposal are:
|
||||
|
||||
- Whether we want to require a lead to approve commit access additions.
|
||||
- We're choosing to do a 6 month inactive period for removal.
|
||||
|
||||
Things I think we should be okay iterating on without going through evolution
|
||||
include:
|
||||
|
||||
- Exactly how we define non-idle activity beyond merging and approving PRs,
|
||||
both of which mechanically require this access.
|
||||
- The detailed process for additions, including nominating.
|
||||
- We want a good starting point, but it may not be worth sending all
|
||||
changes through the proposal process.
|
||||
- The detailed process for removals, such as whether leads are approving
|
||||
removals.
|
||||
- This is something it's been suggested to fully automate, preventing
|
||||
review of removals.
|
||||
|
||||
See the new [Commit access](/docs/project/commit_access.md) document for
|
||||
details.
|
||||
|
||||
## Rationale
|
||||
|
||||
- [Community and culture](/docs/project/goals.md#community-and-culture)
|
||||
- Establishing the processes around commit access, and making sure they're
|
||||
reasonable, is important to maintaining the community.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
### Longer idle times
|
||||
|
||||
We discussed using a longer idle time, like 1, 2, or 3 years. We're leaning
|
||||
towards the shorter 6 month period because of concerns about forgotten access
|
||||
causing issues. We're hoping 6 months is a minimum of inconvenience, and want it
|
||||
to be easy to get access back on request.
|
||||
Reference in New Issue
Block a user