gistfile1.diff

From 7ac97fc5eba0f5d1de4155086383c79aac34f582 Mon Sep 17 00:00:00 2001
From: Tibor Vass <teabee89@gmail.com>
Date: Mon, 26 Jan 2015 18:17:32 -0500
Subject: [PATCH] New pull request review workflow

Signed-off-by: Tibor Vass <teabee89@gmail.com>
---
 project/MAINTAINERS.md | 119 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/project/MAINTAINERS.md b/project/MAINTAINERS.md
index 1a27c92..5ef1763 100644
--- a/project/MAINTAINERS.md
+++ b/project/MAINTAINERS.md
@@ -83,6 +83,125 @@ this (see below "Who decides what?")
     contributor on for weeks or months, having them make many changes to a PR
     that will eventually be rejected.
 
+## Pull request review workflow
+
+The main goal of having a PR review workflow that all maintainers follow
+is to have more consistency across maintainers, have better visibility in
+the pipeline of pull requests and thus increase the efficiency of pull request
+reviews.
+
+Think of this as a flexible workflow that uses common sense to
+bring some useful structure to the review process. If you nevertheless do
+experience problems inherent to this workflow, please discuss with
+maintainers on how to improve and open a pull request to this section.
+
+A pull request can be in 1 of 5 distinct states, for each of which there is a
+corresponding label that needs to be applied. At any step of the way, a PR can
+be closed or return to an earlier state if needed (e.g. back to design review
+during code review).
+
+### States
+
+#### Triage (*no label*)
+
+Maintainers are expected to triage new incoming pull requests by adding
+the correct labels (e.g. `1-design-review`) potentially skipping some steps
+depending on the kind of pull request. Use common sense for judging.
+
+Checking for DCO should be done at this stage.
+
+PRs with new user-facing features should follow all steps detailed below.
+However, performance optimizations or internal features (e.g. changes in
+/project) can skip Design review and/or Documentation review steps.
+
+Bugfixes can skip Design review if the bug is trivial. However, design review
+on bugfixes does not always need to be skipped: if a behavior is being
+mistaken for a bug. Rule of thumb: if you are not sure apply `1-design-review`
+label. If you reproduced the bug yourself, apply `2-code-review` directly.
+
+Proposals with only documentation changes, should get the `1-design-review`
+applied.
+
+Other documentation-only PRs should get the `3-docs-review` label applied
+directly.
+
+If an owner, responsible for closing or merging, can be assigned to the PR,
+the better.
+
+#### Design review (`1-design-review`)
+
+Maintainers are expected to comment on the design of the pull request.
+Review of documentation is expected only in the context of design validation,
+not for stylistic changes.
+
+Ideally, documentation should reflect the expected behavior of the code.
+No code review should take place in this step.
+
+Once design is approved, a maintainer should remove this label and add
+the next appropriate label.
+
+In the case of Proposals with only documentation changes, the `3-docs-review`
+label should be applied.
+
+For changes that involve code, the `2-code-review` label should be applied.
+
+#### Code review (`2-code-review`)
+
+Maintainers are expected to review the code and ensure that it is good
+quality and in accordance with the documentation.
+
+If documentation is absent but expected, maintainers should ask for documentation.
+
+All tests should pass.
+
+Once code is approved (i.e. PR has a certain required number of LGTMs), a
+maintainer should remove this label and add the `3-docs-review` label in the
+case the PR contains changes to the documentation.
+
+For bugfixes or internal changes, the `4-merge` label should be applied.
+
+#### Documentation review (`3-docs-review`)
+
+Maintainers are expected to review the documentation in its bigger context,
+ensuring consistency throughout the entire documentation. They should ask
+for any editorial change that makes the documentation more consistent and
+easier to understand.
+
+Once documentation is approved, a maintainer should remove this label and
+add the `4-merge` label.
+
+#### To be merged (`4-merge`)
+
+Maintainers are expected to merge this pull request as soon as possible.
+They can ask for a rebase, or carry the pull request themselves.
+
+These should be the easy PRs to merge.
+
+### State machine
+
+* `no label` can lead to:
+	* Close (unresponsive contributor without DCO)
+	* Documentation change: `3-docs-review`
+	* Trivial bugfix: `2-code-review`
+	* General case: `1-design-review`
+* `1-design-review` can lead to:
+	* Close (design rejected)
+	* Proposals with only documentation changes: `3-docs-review`
+	* General case: `2-code-review`
+* `2-code-review` can lead to:
+	* Close
+	* Raises design concerns: `1-design-review`
+	* Trivial change not impacting documentation: `4-merge`
+	* General case: `3-docs-review`
+* `3-docs-review` can lead to:
+	* Close
+	* Requires code changes: `2-code-review`
+	* Raises design concerns: `1-design-review`
+	* General case: `4-merge`
+* `4-merge` can lead to:
+	* Close (to carry PR)
+	* Merge
+
 ## Who decides what?
 
 All decisions are pull requests, and the relevant maintainers make