Review Charm for Listing: Charm for CloudCasa

Hello, I reposting here for Bob Adair from CloudCasa: Actually, new users are not allowed to use our review form, because there is the limit of only max. 2 links per post. :neutral_face: That is maybe a bug. - Michael

----snip----

Hello, we from Catalogic Software submit the following charm for review and for listing on charmhub.

Metadata links

CI Links

Documentation Links

Please let us know if something is missing for this review request… Best, Bob Adair

Interesting bug found @mcjaeger! @ali-kelkawi can you please help with this review? You can go through and tick off the items on the checklist available here and post the result in this thread. This prior listing request could serve as an example of how to go about it. Please let me know if you have any questions!

@ali-kelkawi in case you can take over the review, I see just now that the review-request PR is missing. Please hold on, until it is there.

Hi @bobadair I can help you with the review PR. I am sorry, I was so much focused on the link-rewrite issue that I forgot about it to check.

Thanks for posting this for me, Michael! Please let me know what you need for the review.

I’d be happy to help! Is this the review PR?

Oh, good catch, maybe the colleague of Bob has created it already based on our description. Since it contains already corrections to some initial feedback if the charmis ready for review in this commit:

https://github.com/catalogicsoftware/cloudcasa-charm/commit/b1d526ca1c879fb4b113d3d186e84d40a404b078

It should be fine to go with this review pull request. @bobadair apparently your colleague has created this review PR already, good to go also from your side?

Hi. Thanks @ali-kelkawi. Sorry for the delay, but I was waiting for all of the right people on our side to be granted write access to the repo. Yes, one of our folks who had worked on this previously already created the review PR, so we should be able to go ahead and use that as long as it looks ok to you. I marked it as a draft PR so that nobody will try to merge it.

Hello! Please find the review below:

Checked Review item Objective Review criteria Comments
Intended functionality Despite all the items for publication readiness, the charm must work. Charm does what it is meant to do - ideally done in a demo.
Charmhub.io charm detail page A complete and consistent appearance of the charm is required for a quality impression of the charm collection. The overall 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 Generally, the source code for charms must be accessible by the community for transparency and collaboration. It is not entirely mandatory to have the charm published as OSS for review, but the repository must be accessible from the persons working on the review request.
Coding conventions The source code of the charm is accessible in the sense of approachability. Consistent source code style and formatting are also considered a sign of being committed to quality. The implemented checks for coding conventions are reasonable and implemented in the regular CI/CD implementation.
Release automation implementation An implementation for automated releasing to charmhub.io improves the ability to provide updates covering vulnerabilities quickly. Release automation for unstable channels to enable testing when new versions of the charm code or the workload become available. Missing release CI/CD
Unit tests implementation In particular, for the charms review, assuring a reasonable test suite is essential to allow for automated releases in future. The unit tests show relevant coverage. It is a case-dependent review.At the time of review, the test runs successfully. 27% coverage
Unit tests results Availability of test results is mandatory for a working collaborative project. URL to test results from CI/CD automation.
Installation test implemented (could be part of the integration test) In particular, for the charm review, assuring a reasonable test suite is essential to allow for automated releases in future. With this test, for every build, it is ensured that the installation is successful. An implementation for checking the installation is present. The implementation should also check for successful installation as part of the automation, and the workload behaves as expected. At the time of review, the test runs successfully. Missing smoke test
Installation test results Availability of test results is mandatory for a working collaborative project. URL to test results from CI/CD automation. Missing smoke test
Integration tests implemented In particular for the review of charms, assuring a reasonable test suite is important to allow for automated releases in future. With this test, for every build, it is ensured that the integration with other charms is successful. An implementation for testing the required integrations (if applicable) is present. The implementation should also check for successful integration as part of he automation and the workload behaves as expected.
At the time of review, the test runs successfully.
Missing integration tests
Integration test results Availability of test results is mandatory for a working collaborative project. URL to test results from CI/CD automation. Missing integration tests
Documentation for usage The documentation for using the charm should be separate from the documentation for developing or contributing to the charm. URL to this documentation is present.
Documentation for contributing The documentation for contributing to the charm should be separate from the documentation for developing or using the charm. URL to this documentation is present.
Licensing statement For the charm shared, OSS or not, the licensing terms of the charm are clarified (which also implies an identified authorship of the charm). URL to the ruling licensing statement is present.

I have posted a detailed review of the missing elements on this PR.

Thanks for the review @ali-kelkawi. @bobadair Can you please fix the issues listed by Ali and post here again once done?

Yes, thanks for the detailed review, @ali-kelkawi . We will look into the issues.