From 1d1a344eea6e5f111f5102eb12ae51ce59df192d Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Tue, 3 Feb 2015 14:46:28 -0500 Subject: [PATCH 1/4] New pull request workflow Signed-off-by: Tibor Vass Upstream-commit: 14452bcfc2ba0f98987734c0ec00ee439f71a740 Component: engine --- components/engine/MAINTAINERS | 82 +++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/components/engine/MAINTAINERS b/components/engine/MAINTAINERS index b2b12d3c1d..af7c1c3764 100644 --- a/components/engine/MAINTAINERS +++ b/components/engine/MAINTAINERS @@ -124,8 +124,90 @@ the relevant operators. * If the change affects the governance, philosophy, goals or principles of the project, it must be approved by BDFL. + +* 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. `Rules.review.states` contains the list of states with possible targets +for each. """ + # Triage + [Rules.review.states.nolabel] + + # 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. + # + # If an owner, responsible for closing or merging, can be assigned to the PR, + # the better. + + close = "unresponsive contributor without DCO" + 3-docs-review = "Non-proposal documentation-only change" + 2-code-review = "trivial bugfix" + 1-design-review = "general case" + + # Design review + [Rules.review.states.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, this label should be removed and the next label + # added. + + close = "design rejected" + 3-docs-review = "proposals with only documentation changes" + 2-code-review = "general case" + + # Code review + [Rules.review.states.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 according to the rules of the subsystem, this label + # should be removed and the next label added. + + close = "" + 1-design-review = "raises design concerns" + 4-merge = "trivial change not impacting documentation" + 3-docs-review = "general case" + + # Docs review + [Rules.review.states.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, this label should be removed and the next + # label added. + + close = "" + 2-code-review = "requires more code changes" + 1-design-review = "raises design concerns" + 4-merge = "general case" + + # Merge + [Rules.review.states.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. + + close = "carry PR" + merge = "" + [Rules.DCO] title = "Helping contributors with the DCO" From b11be094692bd11268d64a8ce81c54f392287ec2 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Wed, 4 Feb 2015 17:14:15 -0500 Subject: [PATCH 2/4] A few fixes Signed-off-by: Tibor Vass Upstream-commit: 7cbd2d08c0c8b9d6cb2aa2d7e511833bb529462e Component: engine --- components/engine/MAINTAINERS | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/components/engine/MAINTAINERS b/components/engine/MAINTAINERS index af7c1c3764..24fc585658 100644 --- a/components/engine/MAINTAINERS +++ b/components/engine/MAINTAINERS @@ -131,11 +131,12 @@ for each. """ # Triage - [Rules.review.states.nolabel] + [Rules.review.states.0-triage] - # 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. + # Maintainers are expected to triage new incoming pull requests by removing + # the `0-triage` label and 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. # @@ -157,8 +158,8 @@ for each. # Ideally, documentation should reflect the expected behavior of the code. # No code review should take place in this step. # - # Once design is approved, this label should be removed and the next label - # added. + # Once design is approved, a maintainer should make sure to remove this label + # and add the next one. close = "design rejected" 3-docs-review = "proposals with only documentation changes" @@ -168,14 +169,14 @@ for each. [Rules.review.states.2-code-review] # Maintainers are expected to review the code and ensure that it is good - # quality and in accordance with the documentation. + # quality and in accordance with the documentation in the PR. # # If documentation is absent but expected, maintainers should ask for documentation. # # All tests should pass. # - # Once code is approved according to the rules of the subsystem, this label - # should be removed and the next label added. + # Once code is approved according to the rules of the subsystem, a maintainer + # should make sure to remove this label and add the next one. close = "" 1-design-review = "raises design concerns" @@ -190,8 +191,8 @@ for each. # for any editorial change that makes the documentation more consistent and # easier to understand. # - # Once documentation is approved, this label should be removed and the next - # label added. + # Once documentation is approved, a maintainer should make sure to remove this + # label and add the next one. close = "" 2-code-review = "requires more code changes" From 2258e008829519449b449091bf5838143bd8c26b Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Wed, 4 Feb 2015 19:58:31 -0500 Subject: [PATCH 3/4] Clarify docs review role Signed-off-by: Tibor Vass Upstream-commit: 1578f21067b92338c90f15a9a3473137b1db2b99 Component: engine --- components/engine/MAINTAINERS | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/components/engine/MAINTAINERS b/components/engine/MAINTAINERS index 24fc585658..0e91671151 100644 --- a/components/engine/MAINTAINERS +++ b/components/engine/MAINTAINERS @@ -187,9 +187,11 @@ for each. [Rules.review.states.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. + # ensuring consistency, completeness, validity, and breadth of coverage across + # all extent and new 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 make sure to remove this # label and add the next one. From d863024730d0371f9af1e5559f085a08caf3192f Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Thu, 5 Feb 2015 14:04:40 -0500 Subject: [PATCH 4/4] Minor nits Signed-off-by: Tibor Vass Upstream-commit: 30cd215b86337526ce1a7431d8fe4c0f66b5cfff Component: engine --- components/engine/MAINTAINERS | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/engine/MAINTAINERS b/components/engine/MAINTAINERS index 0e91671151..ecad160303 100644 --- a/components/engine/MAINTAINERS +++ b/components/engine/MAINTAINERS @@ -143,9 +143,9 @@ for each. # If an owner, responsible for closing or merging, can be assigned to the PR, # the better. - close = "unresponsive contributor without DCO" - 3-docs-review = "Non-proposal documentation-only change" - 2-code-review = "trivial bugfix" + close = "e.g. unresponsive contributor without DCO" + 3-docs-review = "non-proposal documentation-only change" + 2-code-review = "e.g. trivial bugfix" 1-design-review = "general case" # Design review