Inclusion in Ansible 2.10.x

[felixfontein]

I was looking at this collcetion a bit. I have some questions/points:

1. example in https://github.com/CiscoDevNet/ansible-nso/blob/master/plugins/modules/nso_verify.py does not use FQCN
2. what's https://github.com/CiscoDevNet/ansible-nso/tree/master/tests/sanity/unused_ignores ?
3. do you have a CI running at least the sanity tests with ansible-base's stable-2.10 and devel branches?

Besides that, LGTM.

CC @gundalow @abadger

[gundalow]

If you create a PR with this lines https://github.com/ansible-collections/collection_template/blob/main/.github/workflows/ansible-test.yml#L1-L116 in a new file called .github/workflows/ansible-test.yml then you will get Sanity and Unit tests running.

Any issues please should out and we can help.

[jabelk]

The examples are aligned to the NSO DevNet reservable sandbox instance and don't require a full FQCN

[felixfontein]

@jabelk examples are targeted at end-users, who will not be able to use the modules from the collections when they don't explicitly use FQCNs. I would assume that people will also use this collection outside your reservable sandbox.

Also, this is a mandatory requirement for inclusion in Ansible, see All module and plugin EXAMPLES MUST: in: https://github.com/ansible-collections/overview/blob/main/collection_requirements.rst#documentation

[jabelk]

Ah, I misunderstood. You mean the module call needs the full collection.modulename, not that the network device needs some type of renaming.

[jabelk]

I added cisco.nso. namespace to the module example, thanks!

[mamullen13316]

@felixfontein prior to seeing your last comment on PR #142 I had submitted a new PR ansible/ansible#72753 which I thought was possibly necessary for the migration based on further analysis of the Onyx module migration.

[felixfontein]

@mamullen13316 that's the very last thing that can be merged, and only after (a) this collection is included in Ansible 2.10, and (b) ansible-collections/community.network#142 is merged. (And that PR is only relevant for ansible-core 2.12+.)

[mamullen13316]

@jabelk going through the checklist it seems that we need to convert this repo to using branch name of main instead of master. Would there be impact to others working on this repo, DevNet labs, or anything else that depends on the branch named master being present?

[felixfontein]

@mamullen13316 I don't think that's a requirement for collection repos outside the github.com/ansible-collections namespace. It is nice if collections outside of that namespace also do it, but not required to be part of Ansible.

[mamullen13316]

@felixfontein thanks for the clarification. In that case I believe we have satisfied the requirements in the checklist.

[felixfontein]

@mamullen13316 I think #5 needs to be addressed first.

[felixfontein]

Ok, I think everything's fine now. The only thing that is a bit strange is the https://github.com/CiscoDevNet/ansible-nso/tree/master/tests/sanity/unused_ignores directory, I think it can be safely deleted.

Maybe do another release (1.0.2) so that all fixes are included in the latest release? Then I think this collection can then be added to Ansible 2.10.

@gundalow what do you think?