PR for code review: https://github.com/canonical/livepatch-machine-charm/pull/21
- Publisher: Canonical Commercial Systems
|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.
|Livepatch on Charmhub
|Link to source repository accessible by reviewer
|A reasonable styleguide is enforced in tox.ini/similar and in CI/CD.
tox file + the operator workflow unit tests runs code linting, language, format, and other checks
|Release automation implementation
|CI auto releases charm to edge on merge.
test workflow and the publish workflow
|Unit tests implementation
|Unit tests are run in CI, pass, have at least 50% coverage, and their results are available to the reviewer.
using the operator-workflow test and the unit tests and the results
|Integration tests implemented
|Integration tests for installation, usage and relations are run in CI, pass, and their results are available to the reviewer.
Integration test results. Also The github action is here which runs tests from this directory
|Documentation for usage
|A usage doc exists separate from README.md.
Livepatch docs and discourse tutorial
|Documentation for contributing
|A contributing doc (e.g. CONTRIBUTING.md) exists.
|LICENCE is clearly available to potential users.
- There are other actions in the repo to publish the bundle, promote the charm, publish the charm, etc.
Hello, @0x12b , @varshigupta would you know a colleague who would have time to take over the review of this charm?
Would you mind assisting @mina146 with a charm review using the checklist posted here?
Hello @mina146, in case you were out for a longer break, please do not forget to consider to use the checklist Simon has proposed.
please find the post updated with the requested checklist
any updates on this please? We want to start another charm review for our k8s charm, but we are waiting to see what would be the feedback on this one to resolve comments on both together as they are very similar.
I am sorry, I did not notice that you have used the form already to update the initial post, my bad. I was confusing things. To correct:
- The reviewing person needs to use the form linked at the end of the review process description:
Review request form
The reviewer would use the form that you have provided.
Marc would be the candidate for reviewing the charm.
For now, maybe the input you have provided is sufficient, but the next charm, pls consider using the “Review Request Form”
Hi @marcoppenheimer, maybe there was not much time after your break, but did you have a chance to have a look at reviewing this charm so far?
Hello @mina146, the Publisher name is not ideal for any external persons to understand who “Yellow Squad” is? Who would be behind it? I know that taking a look on launchpad is possible, but at a first glance, the publishing org is not clear.
If that is maintained by us, we would need to have a publisher name indicating “Canonical”. How about “Canonical Commercial Systems” as a publishing group? Or, how would commercial systems like to appear in the public as publishing group?
please find the ownership transfer request here:
many thanks for following this up - do I understand this correctly, the display name would be “Canonical Commercial Systems”?
Have left my review in Github.
- Increasing unit-test coverage
- README and general documentation needs more detail
- Ensuring that publish CI triggers on merge to
for having a bit more status on the thread, could you please let us know if the comments by Marc do make sense to you?