vmware.vmware_rest

[jillr]

Ansible Galaxy: https://galaxy.ansible.com/vmware/vmware_rest
GitHub: https://github.com/ansible-collections/vmware.vmware_rest
Issues Tracker: https://github.com/ansible-collections/vmware.vmware_rest
GitHub Handles: @goneri @Akasurde @jillr
Is the collection part of Automation Hub: Yes
We meet Collection Requirements: Yes

(we're actually missing a CoC link/reference right now - this collection falls under the Ansible CoC and I'll get the README updated shortly.)
ansible-collections/vmware.vmware_rest#208

[tadeboro]

Thank you for your submission, @jillr.

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 and patience.

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
• 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)
  COMMENT: While the aiohttp dependency could be avoided if inclusion requirements would be in place at the time of the initial development, this is unfortunately not the case (collection predates the requirements). So this criteria is fulfilled even without considering the efficiency problems with non-async HTTP clients. Thank you, @jillr, for the explanations.
• 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
  COMMENT: The ignore file is quite large, but all of its content is there because the collection does not support Python < 3.6.
• 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]

I finished an initial review of the collection. Right now, the collection cannot be included in the ansible package.

[tadeboro]

I did a second review and the state is better than it was at the initial review, but there are still a few pieces missing before the collection is ready for inclusion.

[jillr]

Thanks. We will probably not get much work on these until after AnsibleFest (or at least until collections releases and other work needed to be completed before Fest are done).

[tadeboro]

I did another review and things seem to be fine now. But before the inclusion, the collection needs to have a new release with updates, and the colud.common collection needs to make it into the Ansible 5 package.

[Akasurde]

@jillr @tadeboro @goneri I opened ansible-collections/vmware.vmware_rest#230 for addressing requirements.

[jillr]

Periodic CI jobs have been added here ansible/ansible-zuul-jobs#1111

[goneri]

• galaxy.yml and the version bounds: galaxy: set version bounds for cloud.common vmware.vmware_rest#259

[felixfontein]

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
• follows development conventions; as well as these other requirements:
• 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
  MUST FIX The prefix is missing in the examples for example in cluster_moid
• documentation and return sections use version_added: containing the collection version for which an option, module or plugin was added
  MUST FIX The lookup plugins claim to have been added in 2.0.1, which (a) would violate semver and (b) there is no such release; I guess it should be 2.1.0?
• FQCNs are used for extends_documentation_fragment:, unless you are referring to doc_fragments from ansible-core
• MUST FIX The extra collection documentation in docs/docsite/rst/ has some issues:

  ./docs/docsite/rst/guide_vmware_rest.rst:1:0: Label "vmware_rest_ansible" does not start with expected prefix "ansible_collections.vmware.vmware_rest.docsite."
  ./docs/docsite/rst/vmware_rest_scenarios/appliance/appliance_access.rst:1:0: Label "vmware-rest-appliance-access" does not start with expected prefix "ansible_collections.vmware.vmware_rest.docsite."
  ./docs/docsite/rst/vmware_rest_scenarios/appliance/health.rst:1:0: Label "vmware-rest-appliance-health" does not start with expected prefix "ansible_collections.vmware.vmware_rest.docsite."
  ./docs/docsite/rst/vmware_rest_scenarios/appliance/network.rst:1:0: Label "vmware-rest-appliance-network" does not start with expected prefix "ansible_collections.vmware.vmware_rest.docsite."
  ./docs/docsite/rst/vmware_rest_scenarios/appliance/services.rst:1:0: Label "vmware-rest-appliance-services" does not start with expected prefix "ansible_collections.vmware.vmware_rest.docsite."
  ./docs/docsite/rst/vmware_rest_scenarios/appliance/system_managment.rst:1:0: Label "vmware-rest-appliance-shutdown" does not start with expected prefix "ansible_collections.vmware.vmware_rest.docsite."
  ./docs/docsite/rst/vmware_rest_scenarios/vcenter/1_authentication.rst:1:0: Label "vmware-rest-authentication" does not start with expected prefix "ansible_collections.vmware.vmware_rest.docsite."
  ./docs/docsite/rst/vmware_rest_scenarios/vcenter/1_installation.rst:1:0: Label "vmware-rest-installation" does not start with expected prefix "ansible_collections.vmware.vmware_rest.docsite."
  ./docs/docsite/rst/vmware_rest_scenarios/vcenter/2_collect_information.rst:1:0: Label "vmware-rest-collect-info" does not start with expected prefix "ansible_collections.vmware.vmware_rest.docsite."
  ./docs/docsite/rst/vmware_rest_scenarios/vcenter/3_create_vm.rst:1:0: Label "vmware-rest-create-vm" does not start with expected prefix "ansible_collections.vmware.vmware_rest.docsite."
  ./docs/docsite/rst/vmware_rest_scenarios/vcenter/4_vm_info.rst:1:0: Label "vmware-rest-vm-info" does not start with expected prefix "ansible_collections.vmware.vmware_rest.docsite."
  ./docs/docsite/rst/vmware_rest_scenarios/vcenter/5_vm_hardware_tuning.rst:132:0: Label "vmware-rest-attach-a-network" does not start with expected prefix "ansible_collections.vmware.vmware_rest.docsite."
  ./docs/docsite/rst/vmware_rest_scenarios/vcenter/5_vm_hardware_tuning.rst:1:0: Label "vmware-rest-vm-hardware-tuning" does not start with expected prefix "ansible_collections.vmware.vmware_rest.docsite."
  ./docs/docsite/rst/vmware_rest_scenarios/vcenter/6_run_a_vm.rst:1:0: Label "vmware-rest-run-a-vm" does not start with expected prefix "ansible_collections.vmware.vmware_rest.docsite."
  ./docs/docsite/rst/vmware_rest_scenarios/vcenter/7_vm_tool_information.rst:1:0: Label "vmware-rest-vm-tool-information" does not start with expected prefix "ansible_collections.vmware.vmware_rest.docsite."
  ./docs/docsite/rst/vmware_rest_scenarios/vcenter/8_vm_tool_configuration.rst:1:0: Label "vmware-rest-vm-tool-configuration" does not start with expected prefix "ansible_collections.vmware.vmware_rest.docsite."
  

  (pointed out by antsibull-lint collection-docs /path/to/checkout - it would make sense to add this to CI)

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)

[goneri]

2.1.2 has just been release. We now run antsibull-lint in the CI and we use the FQCN in the examples. Can you please refresh the document?

[goneri]

ansible-collections/vmware.vmware_rest#305

[Andersson007]

@felixfontein would you like to update the review?

[felixfontein]

I've updated my review. Right now the collection needs a new release, then everything is fine from my side.

[Andersson007]

When the collection is released, please let me know. I'll create the inclusion topic after that

[felixfontein]

@Andersson007 I checked the commits added to the collection since my last review, everything looks great. This is ready for inclusion!

[jillr]

@tadeboro How do you feel about this collection going into 6.0.0? I'd like to make sure we have plenty of time to make any changes if they are still needed.

[jillr]

It's not clear to me what the next steps are for this ticket. If we're including new collections on minor releases then I assume we won't have major-release specific topics like we did for 5.0.0: ansible-community/community-topics#32. Also we're moving to async over the Wed meeting. Do inclusion candidates need to also have a topic on community-topics that links back to this topic? Has this collection even been approved? I'm assuming vmware.vmware_rest will not be included in 5.4.0 tomorrow but I don't know how I would be sure of that.

[Andersson007]

We've voted on inclusion (i mean it's been a part of the inclusion process). There's also another candidate ready - #39. We should take a look at other collections submitted and we could vote on all ready in one topic.
Generally I'm not sure whether it's worth voting provided that there are two approvals from SC, so maybe we should simplify this stuff not to overwhelm other SC folks.

[Andersson007]

we have also the nautobot collection ready, so we're waiting for release of vmware.vmware_rest and can process them together + community.sap.

[goneri]

I'm not sure why you're waiting for a release but I've recently pushed vmware.vmware_rest 2.1.5.

[felixfontein]

To have a release that actually includes the addressed issues pointed out in the review. It's a bit sad that it took over two months.

Edit: I'm sorry if this felt like an attack, that wasn't my intention. I was really hoping that we could finish the inclusion process for vmware.vmware_rest earlier, every open review that isn't progressing is frustrating, especially when quite some time was spent reviewing.

[mariolenz]

Can someone please close this since vmware.vmware_rest has been added with ansible-community/ansible-build-data#120? I can't, I can only add this comment.

[mariolenz]

Can someone please close this since vmware.vmware_rest has been added with ansible-community/ansible-build-data#120? I can't, I can only add this comment.

Or change the category to Resolved reviews. I don't know how to do this.