standards/review.md - Multiple updates

 * Simplify titles under "Common standards". (Use a noun-phrase instead of an imperative statement.)
 * Combine `r-ext`, `r-core` into r-technical. Add more details. Reference `#gotchas`.
 * Rename `r-read` to `r-code`
 * Rename `rg-schema` to `rg-upgrade`. Add more details.

docs/standards/review.md

@@ -8,8 +8,8 @@ write a long, bespoke blurb.
 
 !!! tip
     The codes below (e.g. `r-jira`) are here to make it easier to reference these standards when chatting with others about PR review.
-    
-### Checklist template {:#checklist} 
+
+### Checklist template {:#checklist}
 
 If you want to quickly indicate which criteria you have evaluated while reviewing a PR, you can copy-paste the following markdown template into your comment and check the boxes as necessary.
 
@@ -27,59 +27,61 @@ If you want to quickly indicate which criteria you have evaluated while reviewin
 
 ## Common standards
 
-### Ensure that the PR links to a JIRA issue. {:#r-jira}
+### JIRA {:#r-jira}
 
 _Standard code: `r-jira`_
 
-For most bug-fixes and improvements, there needs to be a [JIRA issue](/tools/issue-tracking.md#jira). However, small [NFC](/tools/git.md#nfc) PRs and some [WIP](/tools/git.md#wip) PRs may not need that.
+For most bug-fixes and improvements, there needs to be a [JIRA issue](/tools/issue-tracking.md#jira). However, [NFC](/tools/git.md#nfc) and [WIP](/tools/git.md#wip) PRs may not need an issue.
 
-### Examine test results. {:#r-test}
+### Test results {:#r-test}
 
 _Standard code: `r-test`_
 
-If the [automated tests](/testing/continuous-integration.md) comes 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 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.
 
-### Read the code. {:#r-read}
+### Code quality {:#r-code}
 
 _Standard code: `r-read`_
 
 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?
 
-### Try it out. {:#r-run}
+### Run it {:#r-run}
 
 _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.
 
-### Assess impact on users. {:#r-users}
+### User impact {:#r-user}
 
 _Standard code: `r-users`_
 
 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?
 
-### Assess impact on extensions/integrations. {:#r-ext}
-
-_Standard code: `r-ext`_
-
-Would the proposal change the behaviors/inputs/outputs of an API, hook, or widely-used function? If an existing extension uses it, would it continue to work the same way? If you're unsure, consider grepping [universe](/tools/universe.md) for inspiration. If there is a foreseeable problem, has there been commensurate effort to communicate change and provide a fair transition path?
-
-### Assess impact on core. {:#r-core}
+### Technical impact {:#r-technical}
 
-_Standard code: `r-core`_
+_Standard code: `r-technical`_
 
-Would the patch change the contract for a PHP function or JS widget or CSS class? If so, have you verified that there are no stale/dangling references based on the old contract? Look for unexpected side-effects.
+* Would the patch materially change the contract (signature/pre-condition/post-condition) for APIv3, a hook, a PHP function, a PHP class, a JS widget, or a CSS class?
+* Would you consider the changed element to be an officially supported contract? A de-facto important contract? An obscure internal detail?
+* How might the change affect other parts of `civicrm-core`? extensions? third-party integrations?
+* If it's hard to answer, look for inspiration:
+    * Grep `civicrm-core` or [universe](/tools/universe.md) to find out where the API/hook/function/etc is called. Consider how these might be affected.
+    * Look at the [Gotchas](#gotchas) for a list of issues that have been mistakenly overlooked in past reviews.
+* If there is a foreseeable problem:
+    * Is there a simple alternative?
+    * Has there been commensurate effort to communicate change and provide a fair transition path?
 
-### Check for tests or maintainability. {:#r-maint}
+### 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.
 
-### Check for documentation. {:#r-doc}
+### Documentation {:#r-doc}
 
 _Standard code: `r-doc`_
 
-Some changes require addition documentation, or adjustments to existing 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.
+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.
 
 ## Gotchas
 
@@ -110,11 +112,19 @@ _Standard code: `rg-setting`_
 
 If the PR adds or removes a setting, will existing deployments or build-scripts which reference the setting continue to work as expected?
 
-### Schema {:#rg-schema}
+### Upgrades {:#rg-upgrades}
+
+_Standard code: `rg-upgrades`_
+
+Some PRs change the database by modifying schema or inserting new data. Ensure new installations and upgraded installations will end up with consistent schema. (Extra: If it's a backport, take extra care to consider all upgrade paths.)
+
+If the upgrade adds a column or index, use a helper function to insert it. (This makes the upgrade more robust against alternative upgrade paths, such as backports/cherry-picks.)
+
+If the upgrade needs to populate or recompute data on a large table (such as `civicrm_contact`, `civicrm_activity`, or `civicrm_mailing_event_queue`), the upgrade screen could timeout. Consider applying the updates in batches.
 
-_Standard code: `rg-schema`_
+If the upgrade inserts new strings, should they be generated in a multilingual-friendly way?
 
-If the PR changes the DB, ensure new installations and upgraded installations will end up with consistent schema. (Extra: If it's a backport, take extra care to consider all upgrade paths.)
+!!! seealso "See also: [Upgrade Reference](/framework/upgrade)"
 
 ### Hook signature {:#rg-hook}