Reviewing charms

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.
1 Like