Review Template (MC)
(CiviCRM Review Template MC-1.2)
- General standards
- Explain (
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:
- User impact (
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:
- Documentation (
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:
- Run it (
r-run
)- [ ] PASS:
- [ ] ISSUE:
- [ ] COMMENTS:
- Explain (
- Developer standards
- Technical impact (
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:
- Code quality (
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:
- Maintainability (
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:
- Test results (
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:
- Technical impact (