Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cisco NSO module migration #142

Merged
merged 5 commits into from
Dec 7, 2020

Conversation

mamullen13316
Copy link
Contributor

SUMMARY

Migrate the Cisco NSO modules to the cisco.nso namespace

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
  • nso_config
  • nso_verify
  • nso_query
  • nso_action
  • nso_show
ADDITIONAL INFORMATION

The new collection is active on Galaxy here: https://galaxy.ansible.com/cisco/nso and on Github: https://github.com/CiscoDevNet/ansible-nso. This PR is to migrate the modules in community.network over to the new collection. I tried to follow along with the previous Onyx PR (#74 ) which I believe was a similar migration, but please let me know if there is anything that I am missing or needs to be corrected.

Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding negative review to avoid accidental merge.

@mamullen13316
Copy link
Contributor Author

Hello @felixfontein, are there any changes required for this PR to be merged and when might it be merged? Also, will merging this PR take care of re-pointing the documentation on docs.ansible.com to the new collection or is there something separate that needs to be done to accomplish that?

@felixfontein
Copy link
Collaborator

@mamullen13316 the PR is probably merged for 2.0.0, but with some modifications, especially w.r.t the deprecations of the redirects. We're still discussing how to do post-2.10.0 migrations in the community meeting. We've accumulated a bunch of these right now, and I think we're close to finally know how to make the move.

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added affects_2.10 feature This issue/PR relates to a feature request has_issue module module needs_maintainer needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR new_contributor Help guide this first time contributor plugins plugin (any type) stale_ci CI is older than 7 days, rerun before merging labels Nov 16, 2020
@felixfontein
Copy link
Collaborator

For an example of how this PR should look like, see both f896c2986c024436a9e12ea6f9e5bbc828907dc5 (main removal) and dbae7da6bcfeea9c1c64060b577dec72b8f9f2a0 (fixes changelog fragment, removes changelog fragments).

Please note that this can only be merged once the new collection is included in Ansible 2.10.

@mamullen13316
Copy link
Contributor Author

For an example of how this PR should look like, see both f896c2986c024436a9e12ea6f9e5bbc828907dc5 (main removal) and dbae7da6bcfeea9c1c64060b577dec72b8f9f2a0 (fixes changelog fragment, removes changelog fragments).

Please note that this can only be merged once the new collection is included in Ansible 2.10.

Thanks I will review those commits. What needs to be done to have the new collection included in Ansible 2.10?

@felixfontein
Copy link
Collaborator

The conditions on the checklist https://github.com/ansible-collections/overview/blob/main/collection_requirements.rst need to be satisfied, and it needs to be added to the Ansible build system (https://github.com/ansible-community/ansible-build-data/blob/main/2.10/ansible.in). The main ticket for discussing this should be CiscoDevNet/ansible-nso#4

@mamullen13316
Copy link
Contributor Author

Latest commit a2e7066 contains changes to align with f896c2986c024436a9e12ea6f9e5bbc828907dc5 and dbae7da6bcfeea9c1c64060b577dec72b8f9f2a0 as mentioned in the above comments. I also merged the changes with a git pull to my local repo in order to incorporate the changes over the last 28 days, and resolved a merge conflict I had with meta/runtime.yml. Please let me know if this looks ok.

@felixfontein
Copy link
Collaborator

There are multiple nso-related files that are still around:

  1. ./plugins/module_utils/network/nso/nso.py (and the directory containing it)
  2. ./plugins/modules/network/nso/nso_action.py
  3. ./plugins/doc_fragments/nso.py

Also there's no redirect for the doc_fragments plugin (for examples, see https://github.com/ansible-collections/community.general/blob/main/meta/runtime.yml#L503).

@mamullen13316
Copy link
Contributor Author

Not sure how those files came back, they were deleted in the initial commit for this PR (7a0fc45). It looks like maybe during the merge. I checked just to make sure there hadn't been any changes to them since I initially opened the PR, and there haven't been any changes for 3 months. So scratching my head as to why they came back. In any case, this should take care of them as well as adding the redirect for the doc_fragment in meta/runtime.yml.

meta/runtime.yml Outdated Show resolved Hide resolved
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Adding a negative review to prevent premature merge. Once the collection has been added to ansible.in, this can be merged. See here and here for more details.

Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@felixfontein felixfontein added breaking_change This PR contains a breaking change that MUST NOT be backported and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR stale_ci CI is older than 7 days, rerun before merging labels Dec 7, 2020
@felixfontein felixfontein merged commit 8be55e2 into ansible-collections:main Dec 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.10 breaking_change This PR contains a breaking change that MUST NOT be backported feature This issue/PR relates to a feature request has_issue module module needs_maintainer new_contributor Help guide this first time contributor plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants