Hi, We would like the Identity Platform bundle to be searchable and visible in Charmub.
Can you please help us to have it reviewed?
Thanks @natalia-nowakowska for the review request!
@gmerold can you please help with this review? This is the first review request for a bundle that I’ve seen, and I don’t see any others in the topic listing, so we may be breaking some new ground here.
I’m guessing all the individual charms are already listed publicly (if not, then we should start with individual requests for that, I think), and this is specifically for the bundle.
Normally, I say that you can go through and tick off the items on the checklist and post the result in this thread. I think that still applies, but there might be items that are specific to single charms that you should skip. If anything’s unclear, please ask!
Thissingle charm prior listing request could serve as an example of how to go about it, but will need to adapt for being a bundle.
If anything’s unclear, please post or reach out directly and we can figure out what to do. Thanks!
The individual charms from the bundle are already listed: Charmhub | The Open Operator Collection
Hello @tony-meyer.
Thank you for your reply. The mentioned checklist, is it expected from the charm author to post it, or it was your ask for @gmerold to go through it when checking the IdP Bundle?
Hi @pik4ez,
Typically the reviewer posts the checklist. It’s sort of a proof of doing the review.
I’ll get on it as soon as possible.
Hi,
@tony-meyer OK, I went through the checklist and the items seem to work fine for the both charms and the bundles. I’ve made some minor adjustments to reflect the fact that we’re talking about the bundle here.
One thing that is unclear to me is when the checklist mentions URLs availability (for the documentation or the CI/CD results), where exactly do we expect them? In the initial review request post? In the Charmhub listing page? In GitHub? I’ve marked related items as OK because there are some Markdown files in GitHub, but I think it’s a bit of a stretch. A bundle for the entire platform probably deserves a proper Diataxis-compliant documentation, which I don’t see.
The only two items that I’ve marked as NOK are related to the unit tests. I see some Python code in the oauth_tools directory which doesn’t have any unit tests. It would probably make sense to add it.
| Checked | Review item | Objective | Review criteria |
|---|---|---|---|
| ✓ | Intended functionality | Despite all the items for publication readiness, the bundle must work. | Bundle does what it is meant to do - ideally done in a demo. |
| ✓ | Charmhub.io bundle detail page | A complete and consistent appearance of the bundle is required for a quality impression of the bundle 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 bundles must be accessible by the community for transparency and collaboration. | It is not entirely mandatory to have the bundle 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 bundle 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 bundle or the workload become available. |
| X | Unit tests implementation | In particular, for the bundle 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 - it only applies if the bundle repository contains testable code. At the time of review, the test runs successfully. |
| X | 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 bundle 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 bundles, 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 integrations between the charms within the bundle are successful. | An implementation for testing the required integrations is present. The implementation should also check for successful integration as part of the 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 bundle should be separate from the documentation for developing or contributing to the bundle. | URL to this documentation is present. |
| ✓ | Documentation for contributing | The documentation for contributing to the bundle should be separate from the documentation for developing or using the bundle. | URL to this documentation is present. |
| ✓ | Licensing statement | For the bundle shared, OSS or not, the licensing terms of the bundle are clarified (which also implies an identified authorship of the bundle). | URL to the ruling licensing statement is present. |
Thanks @gmerold !
I believe it’s expecting them to be in the request request or otherwise easily found (e.g. it’s a charm with source hosted on GitHub and using GitHub actions for CI). I don’t think it’s intending that they have to be on the Charmhub page, just available for the reviewer.
@natalia-nowakowska what are the documentation plans for this bundle?
@natalia-nowakowska could you address this?
@gmerold @tony-meyer thanks for the review.
The bundle already has a diataxis-compliant documentation, see https://charmhub.io/topics/canonical-identity-platform
The oauth_tools package includes only fixtures and helpers for integration tests, I think unit tests in this case are not necessary.
Perfect :). Could you just hook up the link so that people can easily get there from Charmhub? I assume you can just link to the charmhub topics page rather than to a Discourse one and it will work fine.
That seems fine to me.
Done, thanks for the suggestion.
Can the bundle be published then?
Hi,
Apologies for the delay but I was on parental leave. This is now done.
Thanks,
Odysseus
