New collection review: inspur.ispim

[ispim]

Hello, I'd like to submit the inspur.ispim collection for review:

Ansible Galaxy: https://galaxy.ansible.com/inspur/ispim
Github repo: https://github.com/ispim/inspur.ispim
Github handles: @ispim @baoshanwang

[ispim]

@Andersson007 Could you please help to check whether our new collection meets the requirements?

[Andersson007]

@ispim hello, thank you for submitting a new collection! i'm very busy with other tasks now. When there's a chance, I'll take a look of course.
Maybe someone from the @ansible-collections/steering-committee can help with the first review?

[ispim]

@ansible-collections/steering-committee Could you please help to check whether our collection meets the requirements?

[Andersson007]

Ansible Collections Checklist (short version)

For details about the following points, refer to the Collection Requirements.

Every comment should say whether the reviewer expects it to be addressed, or whether it's optional.

Public availability and communication:

• published on Ansible Galaxy with version 1.0.0 or later
• has a policy of releasing, versioning and deprecation announced to contributors and users in some way
• have a Code of Conduct (CoC) compatible with the Ansible Code of Conduct (CoC)
• have a publicly available issue tracker that does not require a paid level of service to create an account or view issues
• have a public SCM repository, releases of the collection are tagged in this repository

Standards and documentation:

• adheres to semantic versioning
• follows licensing rules
• follows the Ansible documentation standards and the style guide
  MUST FIX: Please add the notes: section to every module with - Supports C(check_mode). or - Does not support C(check_mode).
  SHOULD FIX: If you look at README, you can notice several formatting issues.
• follows development conventions; as well as these other requirements:
  • modules that only gather information are named <something>_info
  • modules that return ansible_facts are named <something>_facts and do not return non-facts
  • other modules must not allow querying information using specific state option values, or similar mechanisms (like state=get or state=query). These features should be moved to <something>_info or <something>_fact modules.
  • check_mode is supported in all _info and _facts modules
• supports Python 2.6 or greater and Python 3.5 or greater. If it does not, read the full guidelines to see if you qualify for an exception and document the unsupported Python versions in the collection README.md and in every module and plugin (or in doc fragments)
  MUST FIX: Please add the requirements: section with ism and Python 3.7+ to the doc fragment
• only uses the allowed plugin types in the plugins/ directory
• has README.md
• documentation, examples, and return sections use FQCNs for the M(..) format macros when referring to modules
• modules (or plugins) from ansible-core use ansible.builtin. as a FQCN prefix
• documentation and return sections use version_added: containing the collection version for which an option, module or plugin was added
• FQCNs are used for extends_documentation_fragment:, unless you are referring to doc_fragments from ansible-core
• public plugins, roles and playbooks do not use files outside of meta/, plugins/, roles/, and playbooks/

Collection management:

• galaxy.yml having tags field set
• collection dependencies must have a lower bound on the version which is at least 1.0.0, and are all part of the ansible package
• meta/runtime.yml defines the minimal version of Ansible which the collection works with
  MUST FIX: in README is said that the collection requires Ansible 2.10 but in meta/runtime.yml is written requires_ansible: '>=2.9.10'.
• has changelog, preferably with changelogs/changelog.yaml
• collection repository should not contain any large objects (binaries) comparatively to the current Galaxy tarball size limit of 20 MB. For example, package installers for testing purposes shouldn't be added.
• collection repository should not contain any unnecessary files like, for example, temporary files created by development tools. Temporary files MUST be added to .gitignore.
  COMMENT: is the bindep.txt file really needed?

Tests:

• passed ansible-test sanity
• if test/sanity/ignore*.txt exists, it does not contain error codes listed here
• has CI tests up and running against each of the "major versions" of ansible-base/ansible-core that the collection supports
  MUST FIX: Please add stable-2.13 to your test matrix
• all CI tests run against every pull request
• all CI tests run against a commit that releases the collection; if they don't pass, the collection won't be released
• all CI tests run regularly (nightly, or at least once per week)

MUST FIX If you and other collection maintainers are not subscribed to the news-for-maintainers repo, please do it not to miss important CI-related updates.

[Andersson007]

@ispim i've finished the first round of review ^, could you please take a look and fix MUST FIXes and comment on COMMENTs/SHOULD FIXes?
Thanks!

[ispim]

Thank you very much. We will start modifying it right away.

[ispim]

@Andersson007 We have modified the problems found. Please check whether they meet the requirements.

[Andersson007]

@ispim it's not an obstacle, just a fyi about README formatting issues:

[Andersson007]

@ispim anyway MUST FIXes LGTM now, so +1 from me for inclusion. I'll change the category that the second review is needed, thanks!

[ispim]

Hello, may I ask when the second system audit will start? Will it be too late to add our collection to Ansible 6 version?

[Andersson007]

Hello @ispim ,

may I ask when the second system audit will start?

I post all the collections waiting for review in Bullhorn every week.
When someone from the Community or Steering Committee decides to conduct the second review, then it'll start. The participation is volunteer.

Will it be too late to add our collection to Ansible 6 version?

No, it won't be late as we can include it in a minor release

[ispim]

Ok, thank you very much.

[Andersson007]

@ansible-collections/steering-committee could anyone please do the second review?

[ispim]

@ansible-collections/steering-committee When can the second review begin?

[Andersson007]

@ispim when there's a volunteer.
I publish a list of the collections that are waiting for inclusion reviews asking for help every week in Bullhorn (though haven't done it during the previous two weeks as was on PTO).
Hope someone will respond ASAP.

[mariolenz]

Ansible Collections Checklist (short version)

For details about the following points, refer to the Collection Requirements.

Every comment should say whether the reviewer expects it to be addressed, or whether it's optional.

Note for reviewers: If you don't know how to check any of the points below, please ask maintainers of the collection you're reviewing or a Steering Committee member for clarifications in comments of corresponding inclusion discussion.

Public availability and communication:

• published on Ansible Galaxy with version 1.0.0 or later
• has a policy of releasing, versioning and deprecation announced to contributors and users in some way
• have a Code of Conduct (CoC) compatible with the Ansible Code of Conduct (CoC)
• have a publicly available issue tracker that does not require a paid level of service to create an account or view issues
• have a public SCM repository, releases of the collection are tagged in this repository

Standards and documentation:

• adheres to semantic versioning
• follows licensing rules
• follows the Ansible documentation standards and the style guide
• follows development conventions; as well as these other requirements:
  • modules that only gather information are named <something>_info
  • modules that return ansible_facts are named <something>_facts and do not return non-facts
  • other modules must not allow querying information using specific state option values, or similar mechanisms (like state=get or state=query). These features should be moved to <something>_info or <something>_fact modules.
  • check_mode is supported in all _info and _facts modules
• supports Python 2.6 or greater and Python 3.5 or greater. If it does not, read the full guidelines to see if you qualify for an exception and document the unsupported Python versions in the collection README.md and in every module and plugin (or in doc fragments)
• only uses the allowed plugin types in the plugins/ directory
• has README.md
• documentation, examples, and return sections use FQCNs for the M(..) format macros when referring to modules
• modules (or plugins) from ansible-core use ansible.builtin. as a FQCN prefix
• documentation and return sections use version_added: containing the collection version for which an option, module or plugin was added
• FQCNs are used for extends_documentation_fragment:, unless you are referring to doc_fragments from ansible-core
• public plugins, roles and playbooks do not use files outside of meta/, plugins/, roles/, and playbooks/

Collection management:

• galaxy.yml having tags field set
• collection dependencies must have a lower bound on the version which is at least 1.0.0, and are all part of the ansible package
• meta/runtime.yml defines the minimal version of Ansible which the collection works with
• has changelog, preferably with changelogs/changelog.yaml
• collection repository should not contain any large objects (binaries) comparatively to the current Galaxy tarball size limit of 20 MB. For example, package installers for testing purposes shouldn't be added.
• collection repository should not contain any unnecessary files like, for example, temporary files created by development tools. Temporary files MUST be added to .gitignore.

Tests:

Note for reviewers: If you don't know how to check the points below, please ask maintainers of the collection you're reviewing how you can do it.

• In most cases, CI is set up via GitHub Actions.
• Check .yml files in the .github/workflows directory. There must be at least the sanity section under jobs containing the ansible-test sanity command running against all supported ansible-core versions that must be also listed there, for example, - stable-2.11, - stable-2.12, - stable-2.13.
• Check workflow runs by clicking the Actions tab in the repository's page - you're interested in Scheduled runs, runs against release commits and runs against pull requests.
• If there are no workflows in the Actions tab, ask the collection maintainers how CI is implemented.

• passed ansible-test sanity
• if test/sanity/ignore*.txt exists, it MUST not contain error codes listed here
• has CI tests up and running against each of the "major versions" of ansible-base/ansible-core that the collection supports
• all CI tests MUST run against every pull request
• all CI tests MUST run regularly (nightly, or at least once per week)
• sanity tests MUST run against a commit that releases the collection; if they don't pass, the collection won't be released

[mariolenz]

@ispim I think you effectively don't test agains stable. It looks like the CI tests against stable are canceled because the one against devel fails with Package 'ansible-core' requires a different Python: 3.8.13 not in '>=3.9' since July 20. But I'm not 100% sure if that is really the root cause.

Apart from this LGTM.

cc @Andersson007

[Andersson007]

@mariolenz thanks much for your review!

[Andersson007]

@ispim please take into consideration mariolenz's comment.

I've changed the status to "In process of inclusion". We'll create a corresponding community topic and will set up a default day when the collection will be included and if there are no objections, we'll do it.

@ispim could you please:

• Subscribe to the news-for-maintainers repo and track announcements there. The ansible-core guys publish changes related to CI (and not only). Otherwise, your CI will get red one day in PRs for unclear reasons.
• Optionally but highly recommended, subscribe to Bullhorn.
• We have the Maintainer guidelines. Might be useful to take a look.

Collections with red CI are considered unmaintained and are removed from the package from time to time, so be sure you and other maintainers are subscribed at least to news-for-maintainers and track it, thanks!

[ispim]

Thank you very much for your advice. I have subscribed to the related project. As for the content of CI test, we have modified it. Please check it. Thank you.

[mariolenz]

@ispim ansible-community/ansible-build-data#158 has been merged.
The collection will appear in the package in the next 6.X minor release (which happens every 3 weeks).
Thanks everyone!