we would like the GLAuth utils charm belonging to the Identity Platform to be searchable /visible in Charmub.
Can you please help us to have it reviewed?
Thanks @shipperizer for the review request! @mmkay 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 be an example of how to go about it.
One section of the review criteria needs improvement:
Charmhub page:
What’s missing:
README.md already has the description and integration guide: https://github.com/canonical/glauth-utils - this should appear as well on the charmhub pageapply-ldif action wording: https://charmhub.io/glauth-utils/actions - minor grammar fixes plus it’s unclear what does this action do. Once the description is here, it might become more clear.Full review results:
| Check result | Review item | Objective | Review criteria |
|---|---|---|---|
| ✓ | 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. |
| x | 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. |
| ✓ | 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. |
| ✓ | 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. |
| ✓ | Installation test results | Availability of test results is mandatory for a working collaborative project. | URL to test results from CI/CD automation. |
| ✓ | 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. |
| ✓ | Integration test results | Availability of test results is mandatory for a working collaborative project. | URL to test results from CI/CD automation. |
| ✓ | 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. |
Hi @mmkay, sorry for going AWOL, we have currently some issues with the integration tests due to an unexpected change in an underlying charm lib Merge pull request #11 from canonical/IAM-837 · canonical/glauth-utils@a9ef184 · GitHub
docs have been updated to reflect the suggestions, we just need to be able to publish them via our workflows, which as said, is currently blocked
bear with us
Hi @mmkay, we’ve published the new revision of the charm. Please take a look again to see if everything is good. Thank you~
One more review round after an offline discussion with @shu-du. Everything now seems good to go, thank you for fixing the issues!
| Check result | Review item | Objective | Review criteria |
|---|---|---|---|
| ✓ | 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. |
| ✓ | 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. |
| ✓ | 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. |
| ✓ | Installation test results | Availability of test results is mandatory for a working collaborative project. | URL to test results from CI/CD automation. |
| ✓ | 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. |
| ✓ | Integration test results | Availability of test results is mandatory for a working collaborative project. | URL to test results from CI/CD automation. |
| ✓ | 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. |