New collection Review: netapp.um_info

[carchi8py]

We've created a few new collections for our other products (same team that currently support the existing NetApp collections). We do monthly update to each collection (And bug fixes through out the month if needed)

Ansible Galaxy: https://galaxy.ansible.com/netapp/um_info
Github: https://github.com/ansible-collections/netapp
Issues Tracker: https://github.com/ansible-collections/netapp/issues
Github Handles: @carchi8py @lonico

[jillr]

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 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
  • Found: Releasing, Versioning and Deprecation netapp#93
• 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
  • Comment: Found GPL v2 in galaxy.yml, MUST be GPL v3 or later https://github.com/ansible-collections/overview/blob/main/collection_requirements.rst#licensing. There's GPL v3 in the modules
• follows the Ansible documentation standards and the style guide
• follows development conventions
• only uses allowed plugin types in the plugins/ directory
• has README.md
• documentation, examples, and return use FQCNs for M(..), examples, and seealso subsections
• modules (or plugins) from ansible-base 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 is used for extends_documentation_fragment:, unless you are referring to doc_fragments from ansible-base

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

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
  • The repo README.md states the included collections support >=2.9 (should probably be 2.9.10). Only found 2.10 testing in the GH Action.
• all CI tests run against every pull request
  • Sanity tests are run, unit tests are commented out. Recommended to be fixed.
• 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)

[jillr]

There is already comments and discussion of the open items on the reviews for other netapp collections contained in the same repo, so I won't repeat those same items here. Those will apply to all of the submitted collections.

[tadeboro]

Just a reminder: collections that want to be part of the Ansible 4.0.0 package need to get approval by 2021-04-26 (full schedule). This inclusion request still has some open issues that need to get addressed by then.

[tadeboro]

Last reminder for inclusion submission

Collections that want to be part of the Ansible 4.0.0 package need to get approval by 2021-04-26, which is just one week away (full schedule). Make sure you submit your collection for the final review a few days before the deadline (reviews take a non-trivial amount of time) and that your collection does meet all criteria listed in the requirements document.

[tadeboro]

Thank you for your submission, @carchi8py.

The items in the checklist can be in one of the following states:

• Items with a checkmark and no attached comment are OK and need no intervention from your side.
• Items with a checkmark that contain a comment will not block the inclusion process, but by following the advice given in the comment, the collection's quality can be improved.
• Items with no checkmark and no extra details are waiting for evaluation.
• Items with no checkmark that contain a comment need your attention.

Feel free to ask any questions and provide feedback. Thanks in advance for your understanding.

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

Public availability and communication:

• published on Ansible Galaxy 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
  Comment: The netapp documentation fragment is invalid: https://github.com/ansible-collections/netapp/blob/main/ansible_collections/netapp/um_info/plugins/doc_fragments/netapp.py#L12-L19, but is not used anywhere. This will not block the inclusion, but should be fixed/simplified in the future.
• follows development conventions
• 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

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

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
• 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)

[tadeboro]

Ugh, my bad. I saw it in https://github.com/ansible-collections/netapp/tree/main/ansible_collections/netapp/cloudmanager#code-of-conduct. Multiple collections is a single repo is killing me. I did the same error in the azure review. Thank you for keeping me honest!

[lonico]

Addressing CoC, FCQNs, and runtime.ml issues.

[lonico]

changes pushed to GitHub.

[tadeboro]

Thank you, @carchi8py, for your perseverance. Once the next release hits Ansible Galaxy, this collection is ready for inclusion as far as I am concerned. Congratulations!

[carchi8py]

@tadeboro the update with these changes are now in galaxy https://galaxy.ansible.com/netapp/um_info

[felixfontein]

• The docs fragment's notes: entry has \\_, which translates to \_ in the documentation (same as New Collection review: netapp.cloudmanager #16 (comment)).
• All modules in this collection are named na_um_list_<object> and seem to be modules that list certain objects. Is there a reason you are not naming them _info, as querying-only modules in Ansible are usually named?
• A notes: saying Supports C(check_mode). would be nice for every module.

[jillr]

@carchi8py After you've moved the filename to start with the _ you can add a deprecated: section to the module docs and a deprecation: block to meta/runtime.yml.
https://docs.ansible.com/ansible/latest/dev_guide/module_lifecycle.html
https://docs.ansible.com/ansible/latest/dev_guide/developing_collections.html#meta-directory

[lonico]

@felixfontein Is it OK if we defer renaming the modules from xxx_list to xxx_info by a few weeks? We'd like to do it as a 21.6.0 release, but we are also restructuring our repos. Or do we have to do it today?

[felixfontein]

Please do not use the _ prefix for deprecating modules, that does not work in collections. The documentation in https://docs.ansible.com/ansible/latest/dev_guide/module_lifecycle.html#deprecating-modules is for modules in the ansible/ansible repository and not for modules in collections. Parts of it also apply to collections (in particular the part on deprecated: and its contents), but neither the underscore nor porting_guide_x.y.rst do apply to collections. (In fact it mentiones a bit below that the documentation needs to be updated for collections, and also mentions routing.yml which was a long time ago renamed to runtime.yml.)

About renaming: please do not deprecate the modules, but rename them and deprecate the old name. For that, basically rename the module, create a symlink from the old name to the new name, and call module.deprecate() in case the module was called with its old name. Also the rename should be mentioned in the documentation (so that users aren't confused). The only reason to deprecate + add new module instead of renaming is when you want to break the module's API, which I think you do not want to do.

Finally: I would probably rename the modules, but wait a bit with deprecation of the old name. This gives users a bit of time where both names work without deprecation, which eases transition for users of your collection.

[jillr]

My mistake, thanks @felixfontein (it's quite really confusing though).

[felixfontein]

I created a PR to update that document here: ansible/ansible#74413

[felixfontein]

This collection has been prepared for inclusion in Ansible 4 in this PR: ansible-community/ansible-build-data#66

[tadeboro]

The netapp.um_info Ansible Collection was accepted into the Ansible community package. The relevant part of the community meeting starts here.

Congratulations!

[felixfontein]

We do expect that the modules are renamed to _info in the next minor release(s) so that in Ansible 4.1.0 the new names will be available. 4.1.0 should be released on June 8th, so if you do this in the 21.6.0 release, everything should be fine (assuming 21.6.0 is released before June 8th).