diff --git a/docs/project/commit_access.md b/docs/project/commit_access.md new file mode 100644 index 000000000000..bf8c72e750f8 --- /dev/null +++ b/docs/project/commit_access.md @@ -0,0 +1,83 @@ +# Commit access + + + + + +## Table of contents + +- [Overview](#overview) +- [Getting access](#getting-access) +- [Removing access](#removing-access) + + + +## 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) diff --git a/docs/project/groups.md b/docs/project/groups.md index 85e418fd54d9..2d32249dddfe 100644 --- a/docs/project/groups.md +++ b/docs/project/groups.md @@ -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) diff --git a/proposals/p4246.md b/proposals/p4246.md new file mode 100644 index 000000000000..faeee18cfb5b --- /dev/null +++ b/proposals/p4246.md @@ -0,0 +1,124 @@ +# Getting commit access + + + +[Pull request](https://github.com/carbon-language/carbon-lang/pull/4246) + + + +## 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) + + + +## 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.