diff --git a/docs/standards/review.md b/docs/standards/review.md
index f1ba68c13b9075de4346d4ac6747ee8469ea4106..9a4e58983077d08815883d2f740d46eaf088a051 100644
--- a/docs/standards/review.md
+++ b/docs/standards/review.md
@@ -16,7 +16,7 @@ You may conduct a structured review, checking each standard in turn. Doing this
* When conducting your first or second structured review, copy [template-del-1.0.md](https://raw.githubusercontent.com/civicrm/civicrm-dev-docs/master/docs/standards/review/template-del-1.0.md) or [template-mc-1.0.md](https://raw.githubusercontent.com/civicrm/civicrm-dev-docs/master/docs/standards/review/template-mc-1.0.md). It provides several examples.
* Once you're familiar with the criteria, copy [template-word-1.0.md](https://raw.githubusercontent.com/civicrm/civicrm-dev-docs/master/docs/standards/review/template-word-1.0.md). It's a bit shorter and quicker.
-## Common standards
+## General standards
### Explanation {:#r-explain}
@@ -35,17 +35,17 @@ __Exception__:
* [WIP](/tools/git.md#wip) PRs do not need a detailed explanation until they're ready for earnest review.
* Genuine [NFC](/tools/git.md#nfc) PRs do not need a detailed explanation.
-### Test results {:#r-test}
+### User impact {:#r-user}
-_Standard code: `r-test`_
+_Standard code: `r-user`_
-If the [automated tests](/testing/continuous-integration.md) come back with any failures, look into why. Hopefully, Jenkins provides an itemized list of failures. If not, dig further into the "Console" output for PHP-fatals or build-failures.
+If a user was comfortable using the old revision, would they upgrade and assimilate naturally and unthinkingly to the new revision? If not, has there been commensurate effort to provide a fair transition-path and communication?
-### Code quality {:#r-code}
+### Documentation {:#r-doc}
-_Standard code: `r-code`_
+_Standard code: `r-doc`_
-Is it understandable? Does it follow common conventions? Does it fit in context? If it changes a difficult section of code -- does it tend to make that section better or worse?
+Some changes require adding or updating documentation. Consider the impact of this change on users, system administrators, and developers. Do they need additional instructions in order to reap the benefits of this change? If so, [update documentation](/documentation/index.md) as necessary by making a corresponding PR on one of the guides.
### Run it {:#r-run}
@@ -53,11 +53,7 @@ _Standard code: `r-run`_
Use the code somehow. You don’t need to attack every imaginable scenario in every PR, but you should do something to try it out. Be proportionate.
-### User impact {:#r-user}
-
-_Standard code: `r-user`_
-
-If a user was comfortable using the old revision, would they upgrade and assimilate naturally and unthinkingly to the new revision? If not, has there been commensurate effort to provide a fair transition-path and communication?
+## Developer standards
### Technical impact {:#r-tech}
@@ -73,17 +69,23 @@ _Standard code: `r-tech`_
* Is there a simple alternative?
* Has there been commensurate effort to communicate change and provide a fair transition path?
+### Code quality {:#r-code}
+
+_Standard code: `r-code`_
+
+Is it understandable? Does it follow common conventions? Does it fit in context? If it changes a difficult section of code -- does it tend to make that section better or worse?
+
### Maintainability {:#r-maint}
_Standard code: `r-maint`_
Many changes should introduce some kind of automated test or protective measure to ensure maintainability. However, there can be tricky cost/benefit issues, and the author and reviewer must exercise balanced judgment.
-### Documentation {:#r-doc}
+### Test results {:#r-test}
-_Standard code: `r-doc`_
+_Standard code: `r-test`_
-Some changes require adding or updating documentation. Consider the impact of this change on users, system administrators, and developers. Do they need additional instructions in order to reap the benefits of this change? If so, [update documentation](/documentation/index.md) as necessary by making a corresponding PR on one of the guides.
+If the [automated tests](/testing/continuous-integration.md) come back with any failures, look into why. Hopefully, Jenkins provides an itemized list of failures. If not, dig further into the "Console" output for PHP-fatals or build-failures.
## Gotchas
diff --git a/docs/standards/review/template-del-1.0.md b/docs/standards/review/template-del-1.0.md
index 0c667d26df4ed0600313fe3978e0bc2d163b0c39..3c8edd5fb6428658136c375f8214ff07c27ba659 100644
--- a/docs/standards/review/template-del-1.0.md
+++ b/docs/standards/review/template-del-1.0.md
@@ -1,53 +1,55 @@
-(*CiviCRM Review Template DEL-1.1*)
+(*CiviCRM Review Template DEL-1.2*)
<!-- In each category, choose the option that most applies. Delete the others. Optionally, provide more details or explanation in the "Comments". -->
-
-* Explain ([`r-explain`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-explain))
- * __UNREVIEWED__
- * __PASS__ : The goal/problem/solution have been adequately explained in the PR.
- * __PASS__ : The goal/problem/solution have been adequately explained with a link (JIRA, Github, Gitlab, StackExchange).
- * __ISSUE__: Please provide a better explanation of the goal/problem being addressed.
- * __ISSUE__: Please provide a better explanation of how this solution works.
- * __COMMENTS__: <!-- optional -->
-* Test results ([`r-test`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-test))
- * __UNREVIEWED__
- * __PASS__: The test results are all-clear.
- * __PASS__: The test results have failures, but these have been individually inspected and found to be irrelevant.
- * __ISSUE__: The test failures need to be resolved.
- * __COMMENTS__: <!-- optional -->
-* Code quality ([`r-code`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-code))
- * __UNREVIEWED__
- * __PASS__: The functionality, purpose, and style of the code seems clear+sensible.
- * __ISSUE__: Something was unclear to me.
- * __ISSUE__: The approach should be different.
- * __COMMENTS__: <!-- optional -->
-* Documentation ([`r-doc`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-doc))
- * __UNREVIEWED__
- * __PASS__: There are relevant updates for the documentation.
- * __PASS__: The changes do not require documentation.
- * __ISSUE__: The user documentation should be updated.
- * __ISSUE__: The administrator documentation should be updated.
- * __ISSUE__: The developer documentation should be updated.
- * __COMMENTS__: <!-- optional -->
-* Maintainability ([`r-maint`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-maint))
- * __UNREVIEWED__
- * __PASS__: The change sufficiently improves test coverage, or the change is trivial enough that it does not require tests.
- * __PASS__: The change does not sufficiently improve test coverage, but special circumstances make it important to accept the change anyway.
- * __ISSUE__: The change does not sufficiently improve test coverage.
- * __COMMENTS__: <!-- optional -->
-* Run it ([`r-run`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-run))
- * __UNREVIEWED__
- * __PASS__: <!-- describe how you ran it -->
- * __ISSUE__: <!-- describe how you ran it -->
-* User impact ([`r-user`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-user))
- * __UNREVIEWED__
- * __PASS__: The change would be intuitive or unnoticeable for a majority of users who work with this feature.
- * __ISSUE__: The change would noticeably impact the user-experience (eg requiring retraining).
- * __PASS__: The change would noticeably impact the user-experience (eg requiring retraining), but this has been addressed with a suitable transition/communication plan.
- * __COMMENTS__: <!-- optional -->
-* Technical impact ([`r-tech`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-tech))
- * __UNREVIEWED__
- * __PASS__: The change preserves compatibility with existing callers/code/downstream.
- * __PASS__: The change potentially affects compatibility, but the risks have been sufficiently managed.
- * __ISSUE__: The change potentially affects compatibility, and the risks have **not** been sufficiently managed.
- * __COMMENTS__: <!-- optional -->
+* General standards
+ * Explain ([`r-explain`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-explain))
+ * __UNREVIEWED__
+ * __PASS__ : The goal/problem/solution have been adequately explained in the PR.
+ * __PASS__ : The goal/problem/solution have been adequately explained with a link (JIRA, Github, Gitlab, StackExchange).
+ * __ISSUE__: Please provide a better explanation of the goal/problem being addressed.
+ * __ISSUE__: Please provide a better explanation of how this solution works.
+ * __COMMENTS__: <!-- optional -->
+ * User impact ([`r-user`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-user))
+ * __UNREVIEWED__
+ * __PASS__: The change would be intuitive or unnoticeable for a majority of users who work with this feature.
+ * __ISSUE__: The change would noticeably impact the user-experience (eg requiring retraining), and the approach should be changed.
+ * __ISSUE__: The change would noticeably impact the user-experience (eg requiring retraining), and we need a better transition/communication plan.
+ * __PASS__: The change would noticeably impact the user-experience (eg requiring retraining), but this has been addressed with a suitable transition/communication plan.
+ * __COMMENTS__: <!-- optional -->
+ * Documentation ([`r-doc`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-doc))
+ * __UNREVIEWED__
+ * __PASS__: There are relevant updates for the documentation.
+ * __PASS__: The changes do not require documentation.
+ * __ISSUE__: The user documentation should be updated.
+ * __ISSUE__: The administrator documentation should be updated.
+ * __ISSUE__: The developer documentation should be updated.
+ * __COMMENTS__: <!-- optional -->
+ * Run it ([`r-run`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-run))
+ * __UNREVIEWED__
+ * __PASS__: <!-- describe how you ran it -->
+ * __ISSUE__: <!-- describe how you ran it -->
+* Developer standards
+ * Technical impact ([`r-tech`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-tech))
+ * __UNREVIEWED__
+ * __PASS__: The change preserves compatibility with existing callers/code/downstream.
+ * __PASS__: The change potentially affects compatibility, but the risks have been sufficiently managed.
+ * __ISSUE__: The change potentially affects compatibility, and the risks have **not** been sufficiently managed.
+ * __COMMENTS__: <!-- optional -->
+ * Code quality ([`r-code`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-code))
+ * __UNREVIEWED__
+ * __PASS__: The functionality, purpose, and style of the code seems clear+sensible.
+ * __ISSUE__: Something was unclear to me.
+ * __ISSUE__: The approach should be different.
+ * __COMMENTS__: <!-- optional -->
+ * Maintainability ([`r-maint`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-maint))
+ * __UNREVIEWED__
+ * __PASS__: The change sufficiently improves test coverage, or the change is trivial enough that it does not require tests.
+ * __PASS__: The change does not sufficiently improve test coverage, but special circumstances make it important to accept the change anyway.
+ * __ISSUE__: The change does not sufficiently improve test coverage.
+ * __COMMENTS__: <!-- optional -->
+ * Test results ([`r-test`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-test))
+ * __UNREVIEWED__
+ * __PASS__: The test results are all-clear.
+ * __PASS__: The test results have failures, but these have been individually inspected and found to be irrelevant.
+ * __ISSUE__: The test failures need to be resolved.
+ * __COMMENTS__: <!-- optional -->
diff --git a/docs/standards/review/template-mc-1.0.md b/docs/standards/review/template-mc-1.0.md
index ff49ed25b7d0156d151c6b70084a78e148c81f00..66670349d64e1471a0989c51d4b75103ff752969 100644
--- a/docs/standards/review/template-mc-1.0.md
+++ b/docs/standards/review/template-mc-1.0.md
@@ -1,45 +1,47 @@
-(*CiviCRM Review Template MC-1.1*)
+(*CiviCRM Review Template MC-1.2*)
<!-- In each category, choose the option that most applies. Optionally, provide more details or explanation in the "Comments". -->
-
-* Explain ([`r-explain`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-explain))
- * [ ] __PASS__ : The goal/problem/solution have been adequately explained in the PR.
- * [ ] __PASS__ : The goal/problem/solution have been adequately explained with a link (JIRA, Github, Gitlab, StackExchange).
- * [ ] __ISSUE__: Please provide a better explanation of the goal/problem being addressed.
- * [ ] __ISSUE__: Please provide a better explanation of how this solution works.
- * [ ] __COMMENTS__: <!-- optional -->
-* Test results ([`r-test`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-test))
- * [ ] __PASS__: The test results are all-clear.
- * [ ] __PASS__: The test results have failures, but these have been individually inspected and found to be irrelevant.
- * [ ] __ISSUE__: The test failures need to be resolved.
- * [ ] __COMMENTS__: <!-- optional -->
-* Code quality ([`r-code`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-code))
- * [ ] __PASS__: The functionality, purpose, and style of the code seems clear+sensible.
- * [ ] __ISSUE__: Something was unclear to me.
- * [ ] __ISSUE__: The approach should be different.
- * [ ] __COMMENTS__: <!-- optional -->
-* Documentation ([`r-doc`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-doc))
- * [ ] __PASS__: There are relevant updates for the documentation, or the changes do not require documentation.
- * [ ] __ISSUE__: The user documentation should be updated.
- * [ ] __ISSUE__: The administrator documentation should be updated.
- * [ ] __ISSUE__: The developer documentation should be updated.
- * [ ] __COMMENTS__: <!-- optional -->
-* Maintainability ([`r-maint`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-maint))
- * [ ] __PASS__: The change sufficiently improves test coverage, or the change is trivial enough that it does not require tests.
- * [ ] __PASS__: The change does not sufficiently improve test coverage, but special circumstances make it important to accept the change anyway.
- * [ ] __ISSUE__: The change does not sufficiently improve test coverage.
- * [ ] __COMMENTS__: <!-- optional -->
-* Run it ([`r-run`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-run))
- * [ ] __PASS__: <!-- describe how you ran it -->
- * [ ] __ISSUE__: <!-- describe how you ran it -->
- * [ ] __COMMENTS__: <!-- optional -->
-* User impact ([`r-user`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-user))
- * [ ] __PASS__: The change would be intuitive or unnoticeable for a majority of users who work with this feature.
- * [ ] __ISSUE__: The change would noticeably impact the user-experience (eg requiring retraining).
- * [ ] __PASS__: The change would noticeably impact the user-experience (eg requiring retraining), but this has been addressed with a suitable transition/communication plan.
- * [ ] __COMMENTS__: <!-- optional -->
-* Technical impact ([`r-tech`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-tech))
- * [ ] __PASS__: The change preserves compatibility with existing callers/code/downstream.
- * [ ] __PASS__: The change potentially affects compatibility, but the risks have been sufficiently managed.
- * [ ] __ISSUE__: The change potentially affects compatibility, and the risks have **not** been sufficiently managed.
- * [ ] __COMMENTS__: <!-- optional -->
+* General standards
+ * Explain ([`r-explain`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-explain))
+ * [ ] __PASS__ : The goal/problem/solution have been adequately explained in the PR.
+ * [ ] __PASS__ : The goal/problem/solution have been adequately explained with a link (JIRA, Github, Gitlab, StackExchange).
+ * [ ] __ISSUE__: Please provide a better explanation of the goal/problem being addressed.
+ * [ ] __ISSUE__: Please provide a better explanation of how this solution works.
+ * [ ] __COMMENTS__: <!-- optional -->
+ * User impact ([`r-user`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-user))
+ * [ ] __PASS__: The change would be intuitive or unnoticeable for a majority of users who work with this feature.
+ * [ ] __ISSUE__: The change would noticeably impact the user-experience (eg requiring retraining), and the approach should be changed.
+ * [ ] __ISSUE__: The change would noticeably impact the user-experience (eg requiring retraining), and we need a better transition/communication plan.
+ * [ ] __PASS__: The change would noticeably impact the user-experience (eg requiring retraining), but this has been addressed with a suitable transition/communication plan.
+ * [ ] __COMMENTS__: <!-- optional -->
+ * Documentation ([`r-doc`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-doc))
+ * [ ] __PASS__: There are relevant updates for the documentation, or the changes do not require documentation.
+ * [ ] __ISSUE__: The user documentation should be updated.
+ * [ ] __ISSUE__: The administrator documentation should be updated.
+ * [ ] __ISSUE__: The developer documentation should be updated.
+ * [ ] __COMMENTS__: <!-- optional -->
+ * Run it ([`r-run`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-run))
+ * [ ] __PASS__: <!-- describe how you ran it -->
+ * [ ] __ISSUE__: <!-- describe how you ran it -->
+ * [ ] __COMMENTS__: <!-- optional -->
+* Developer standards
+ * Technical impact ([`r-tech`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-tech))
+ * [ ] __PASS__: The change preserves compatibility with existing callers/code/downstream.
+ * [ ] __PASS__: The change potentially affects compatibility, but the risks have been sufficiently managed.
+ * [ ] __ISSUE__: The change potentially affects compatibility, and the risks have **not** been sufficiently managed.
+ * [ ] __COMMENTS__: <!-- optional -->
+ * Code quality ([`r-code`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-code))
+ * [ ] __PASS__: The functionality, purpose, and style of the code seems clear+sensible.
+ * [ ] __ISSUE__: Something was unclear to me.
+ * [ ] __ISSUE__: The approach should be different.
+ * [ ] __COMMENTS__: <!-- optional -->
+ * Maintainability ([`r-maint`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-maint))
+ * [ ] __PASS__: The change sufficiently improves test coverage, or the change is trivial enough that it does not require tests.
+ * [ ] __PASS__: The change does not sufficiently improve test coverage, but special circumstances make it important to accept the change anyway.
+ * [ ] __ISSUE__: The change does not sufficiently improve test coverage.
+ * [ ] __COMMENTS__: <!-- optional -->
+ * Test results ([`r-test`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-test))
+ * [ ] __PASS__: The test results are all-clear.
+ * [ ] __PASS__: The test results have failures, but these have been individually inspected and found to be irrelevant.
+ * [ ] __ISSUE__: The test failures need to be resolved.
+ * [ ] __COMMENTS__: <!-- optional -->
diff --git a/docs/standards/review/template-word-1.0.md b/docs/standards/review/template-word-1.0.md
index 1f187414f96ab199fcfa327ac2775748d87507cc..3711d6e2878c36b7f53cffa6c922cc1da626fe40 100644
--- a/docs/standards/review/template-word-1.0.md
+++ b/docs/standards/review/template-word-1.0.md
@@ -1,12 +1,14 @@
-(*CiviCRM Review Template WORD-1.1*)
+(*CiviCRM Review Template WORD-1.2*)
<!-- In each category, change the word "Undecided" to "Pass" or "Issue". Add explanatory comments if prompted or desired. -->
-* ([`r-explain`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-explain)) __Undecided__
-* ([`r-test`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-test)) __Undecided__
-* ([`r-code`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-code)) __Undecided__
-* ([`r-doc`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-doc)) __Undecided__
-* ([`r-maint`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-maint)) __Undecided__
-* ([`r-run`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-run)) __Undecided__: <!-- Describe -->
-* ([`r-user`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-user)) __Undecided__: <!-- Describe -->
-* ([`r-tech`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-tech)) __Undecided__: <!-- Describe -->
+* General standards
+ * ([`r-explain`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-explain)) __Undecided__
+ * ([`r-user`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-user)) __Undecided__: <!-- Describe -->
+ * ([`r-doc`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-doc)) __Undecided__
+ * ([`r-run`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-run)) __Undecided__: <!-- Describe -->
+* Developer standards
+ * ([`r-tech`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-tech)) __Undecided__: <!-- Describe -->
+ * ([`r-code`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-code)) __Undecided__
+ * ([`r-maint`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-maint)) __Undecided__
+ * ([`r-test`](https://docs.civicrm.org/dev/en/latest/standards/review/#r-test)) __Undecided__