Proposal for an explicit GitHub workflow.

CONTRIBUTING.md

@@ -245,6 +245,13 @@ Every file type uses a variation on the same license text ("Apache-2.0 WITH
 LLVM-exception") with similar formatting. If you're not sure what text to use,
 please ask on Discourse Forums.
 
+## Workflow
+
+Carbon repositories all follow a common
+[pull-request workflow](docs/project/pull_request_workflow.md) for landing
+changes. It is a trunk-based development model that emphasizes small,
+incremental changes and preserves a simple linear history.
+
 ## Acknowledgements
 
 Carbon's Contributing guidelines are based on

docs/project/pull_request_workflow.md

@@ -0,0 +1,116 @@
+# Trunk-based pull-request GitHub workflow
+
+<!--
+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
+-->
+
+Carbon repositories follow a few basic principles:
+
+- Development directly on the `trunk` branch and
+  [revert to green](#green-tests).
+- Always use pull requests, rather than pushing directly.
+- Changes should be small, incremental, and review-optimized.
+- Preserve linear history by
+  [rebasing](https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/about-pull-request-merges#rebase-and-merge-your-pull-request-commits)
+  or
+  [squashing](https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/about-pull-request-merges#squash-and-merge-your-pull-request-commits)
+  pull requests rather than using unsquashed merge commits.
+
+These principles try to optimize for several different uses or activities with
+version control:
+
+- Continuous integration and bisection to identify failures and revert to green.
+- Code review both at the time of commit and follow-up review after commit.
+- Understanding how things evolve over time, which can manifest in different
+  ways:
+  - When were things introduced?
+  - How does the main branch and project evolve over time?
+  - How was a bug or surprising thing introduced?
+
+Note that this isn't a complete guide to doing code reviews, and just focuses on
+the mechanical workflow and branch management. TODO: Add an explicit link to
+more detailed guidance on managing pull request based code reviews when it is
+developed.
+
+## Trunk based development
+
+We work in a simple
+[trunk-based development](https://trunkbaseddevelopment.com/) model. This means
+all development activity takes place on a single common `trunk` branch in the
+repository (our default branch). We focus on
+[small, incremental changes](#small_incremental_changes) rather than feature
+branches or the "scaled" variations of this workflow.
+
+### Green tests
+
+The `trunk` branch should always stay "green". That means that if tests fail or
+if we discover bugs or errors, we revert to a "green" state by default, where
+the failure or bug is no longer present. Fixing forward is fine if that will be
+comparably fast and efficient. The goal isn't to dogmatically avoid fixing
+forward, but to prioritize getting back to green quickly. We hope to eventually
+tool this through automatic continuous-integration powered submit queues, but
+even those can fail and the principle remains.
+
+## Always use pull requests (with review) rather than pushing directly
+
+We want to ensure that changes to Carbon are always reviewed, and the simplest
+way to do this is to consistently follow a pull request workflow. Even if the
+change seems trivial, still go through a pull request -- it'll likely be trivial
+to review. Always wait for someone else to review your pull request rather than
+just merging it, even if you have permission to do so.
+
+Our GitHub repos are configured to require pull requests and review before they
+are merged, so this rule is enforced automatically.
+
+## Small, incremental changes
+
+Developing in small, incremental changes improves code review time, continuous
+integration, and bisection. This means we typically squash pull requests into a
+single commit when landing. We use two fundamental guides for deciding how to
+split up pull requests:
+
+1. Ensure that each pull request builds and passes any tests cleanly when you
+   request review and when it lands. This will ensure bisection and continuous
+   integration can effectively process them.
+
+2. Without violating the first point, try to get each pull request to be "just
+   right": not too big, not too small. You don't want to separate a pattern of
+   tightly related changes into separate requests when they're easier to review
+   as a set or batch, and you don't want to bundle unrelated changes together.
+   Typically you should try to keep the pull request as small as you can without
+   breaking apart tightly coupled changes. However, listen to your code reviewer
+   if they ask to split things up or combine them.
+
+While the default is to squash pull requests into a single commit, _during_ the
+review you typically want to leave the development history undisturbed until the
+end so that comments on any particular increment aren't lost. We typically use
+the GitHub squash-and-merge functionality to land things.
+
+### Managing pull requests with multiple commits
+
+Sometimes, it will make sense to _land_ a series of separate commits for a
+single pull request through rebasing. This can happen when there is important
+overarching context that should feed into the review, but the changes can be
+usefully decomposed when landing them. When following this model, each commit
+you intend to end up on the `trunk` branch needs to follow the same fundamental
+rules as the pull request above: they should each build and pass tests when
+landed in order, and they should have well written, cohesive commit messages.
+
+Prior to landing the pull request, you are expected to rebase it to produce this
+final commit sequence, either interactively or not. This kind of rebase rewrites
+the history in Git, which can make it hard to track the resolution of code
+review comments. Typically, only do this as a cleanup step when the review has
+finished, or when it won't otherwise disrupt code review. It is healthy and
+expected to add "addressing review comments" commits during the review and then
+squashing them away before the pull request is merged.
+
+## Linear history
+
+We want the history of the `trunk` branch of each repository to be as simple and
+easy to understand as possible. While Git has strong support for managing
+complex history and merge patterns, we find understanding and reasoning about
+the history -- especially for humans -- to be at least somewhat simplified by
+sticking to a linear progression. As a consequence, we either squash pull
+requests or rebase them when merging them.

proposals/p0029.md

@@ -0,0 +1,157 @@
+# Linear, rebase, and pull-request GitHub workflow
+
+<!--
+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/29)
+
+## Problem
+
+There are a variety of workflows that can be effectively used with both Git
+version control and GitHub. We should have a common and consistent workflow
+across as much of Carbon as possible. While some specific areas may end up
+needing specialized flows to suit their needs, these should be the exceptions
+and can be handled on a case-by-case basis when they arise.
+
+## Background
+
+- Chapter 16 "Version Control and Branch Management" in the SWE book
+  (_[Software Engineering at Google](https://www.amazon.com/Software-Engineering-Google-Lessons-Programming/dp/1492082791)_)
+- [Trunk Based Development](https://trunkbaseddevelopment.com/)
+- GitHub documentation on
+  "[pull requests](https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/about-pull-requests)"
+  - [Configuration of their merges](https://help.github.com/en/github/administering-a-repository/configuring-pull-request-merges)
+  - [Protected branches](https://help.github.com/en/github/administering-a-repository/about-protected-branches)
+
+## Proposal
+
+Carbon repositories should all follow a common
+[workflow](/docs/project/pull_request_workflow.md) for managing and landing
+changes. The document outlines the specifics of the proposed workflow as well as
+the core motivation.
+
+### Rename the default git branch to `trunk`
+
+This achieves two goals:
+
+1. Replaces the term `master`. This term, while only used in isolation in Git,
+   [was used](https://mail.gnome.org/archives/desktop-devel-list/2019-May/msg00066.html)
+   in immediately preceding and related systems as part of extremely problematic
+   "master/slave" terminology. That background associates the term with
+   unacceptable historical and cultural meanings. The intent of those using or
+   adopting the term isn't relevant to this association. The less overtly
+   problematic term being isolated from the rest doesn't erase its history, and
+   doesn't completely avoid painful associations.
+
+2. It directly anchors and reinforces contributors on the trunk-based workflow.
+
+### Longer discussion of linear history
+
+While Git can effectively bisect through complex history graphs, this is
+significantly harder than bisecting across linear history. Especially for any
+part of Carbon involving code, we expect bisection to be a pervasive tool and we
+can make it simpler and more effective by forcing a linear history.
+
+A linear history also makes it much easier to ask questions about whether a
+particular change has landed yet, or when a bug was introduced. For some people,
+releases are more simply understood as branching from a specific snapshot of the
+linear history. While tools like `git log` can provide similar functionality, it
+is less trivially understood.
+
+Continuous integration is simplified for many of the same reasons as bisection:
+the set of potential deltas is reduced to a linear sequence. Reverting to green
+becomes easier to understand, and testing each incremental commit has a single
+obvious interpretation.
+
+Requiring linear history also incentivises incremental development that is
+committed early to the main branch. This, in essence, ensures a single source of
+truth model even with the distributed version control system. Because
+works-in-progress are required to be rebased, they tend to merge early and often
+rather than forming long-lived development branches. This helps reduce the total
+merge resolution and testing costs as a project scales. For more details about
+the advantages of using a single source of truth, see the full text of the
+"Version Control and Branch Management" chapter in the SWE book.
+
+One concern with linear history when rebasing a sequence of changes and merging
+them is that the pull request associated with that sequence might not be obvious
+from the main branch commit log. However, there is enough information in the
+repository to establish the relationship, and GitHub's UI surfaces the pull
+request on each commit in the series.
+
+## Alternatives considered
+
+### LLVM model
+
+LLVM allows directly pushing/submitting to their "trunk" with post-commit
+review. LLVM enforces linear history for day-to-day development. Merge commits
+are allowed for rare events like contributing a new subproject.
+
+Pros:
+
+- Still has linear history.
+- Incentivizes squashing for continuous integration and bisection.
+- Very low overhead for fixing trivial mistakes.
+
+Cons:
+
+- Creates extremely bad incentives around code review.
+  - Lots of patches don't get pre-commit review, even if they would benefit from
+    it.
+  - Very experienced contributors are much better at avoiding pre-commit review,
+    so are rarely blocked waiting on review.
+  - Leads to the most experienced members of the community not doing enough code
+    reviews, or being timely enough in code reviews.
+  - Lots of patches submitted with post-commit review are never reviewed in
+    practice unless they break something.
+- UI and basic support for code reviews entirely focused on pull requests.
+
+### Fork and merge model
+
+Classically, Git and GitHub support merging entire complex graphs of
+development.
+
+Pros:
+
+- Mostly supported by pull requests, so still able to use much of that
+  functionality.
+- Supports a model in which contributors do not communicate and can each develop
+  a local, decentralized fork while still achieving overall reconciliation.
+- Can model much more complex history of code evolution faithfully in the
+  tooling.
+  - Most of the time these aren't so complex that they create problems for
+    humans.
+
+Cons:
+
+- History is harder for humans to understand and reason about in complex cases.
+- Bisection and continuous integration are more complex.
+  - May create difficulty for continuous integration against mainline, because
+    unclear what "order" they should be applied / explored. While there are
+    technical approaches to address this, the don't seem to eliminate the
+    complexity, merely provide a clear set of mechanics for handling it.
+- Reduces incentives to land small, incremental changes by allowing
+  non-linearity to reduce the effort required for large and/or complex merges.
+- Makes review of the main branch's history harder due to non-linearity.
+
+### Fork and merge, but branches can only have linear history
+
+Imagine a fork and merge model, but PRs can only have linear history. That is,
+branching and merging within a PR, or merging `trunk` into PR is not allowed. In
+this model, the only merge commits are merges of PRs into `trunk`. PRs
+themselves don’t contain merge commits.
+
+Pros:
+
+- Mostly supported by GitHub pull requests, so still able to use much of that
+  functionality.
+- Restricts non-linearity of history. The only non-linearity that is left is
+  merge commits on the trunk branch. PRs themselves can’t contain merge commits.
+
+Cons:
+
+- Requires a custom presubmit on GitHub that checks linearity of a PR.
+- The cons of the fork and merge strategy regarding the complexity of history
+  remain, but to a smaller degree, since non-linearity is restricted.