This is great @mcjaeger!
I just completed a review using this and had a few conclusions:
- I find the checklist imposes a high cognitive load on the reviewer: I need to unabstract the abstractions.
- I believe some items in the table could be consolidated.
The checklist above is great for rationale, but I think for actual reviews we could use something more lightweight, wdyt?
For example (usage example):
Review item | Review criteria | Evidence / notes |
---|---|---|
Charmhub.io charm detail page | The overall charmhub.io appearance looks good, which means: * The name complies with the naming guidelines. * The publisher is identified. * The links are provided. * The documentation looks reasonable. | |
Source repository | Link to source repository accessible by reviewer | |
Coding conventions | A reasonable styleguide is enforced in tox.ini/similar and in CI/CD. | |
Release automation implementation | CI auto releases charm to edge on merge. | |
Unit tests implementation | Unit tests are run in CI, pass, have at least 50% coverage, and their results are available to the reviewer. | |
Integration tests implemented | Integration tests for installation, usage and relations are run in CI, pass, and their results are available to the reviewer. | |
Documentation for usage | A usage doc exists separate from README.md. | |
Documentation for contributing | A contributing doc (e.g. CONTRIBUTING.md) exists. | |
Licensing statement | LICENCE is clearly available to potential users. |