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

Remove DellEMC collections from community.general collectons #948

Closed
wants to merge 9 commits into from
Closed

Conversation

rajeevarakkal
Copy link
Contributor

SUMMARY

Need to get removed the dellemc collections community general repository as dellemc all modules are now available in Galaxy dellemc.openmanage collection.

ISSUE TYPE
  • Feature Pull Request
ADDITIONAL INFORMATION
Already a PR created to update the botmeta & runtime files 
https://github.com/ansible/ansible/pull/71875

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 prevent premature merge. dellemc.openmanage is not included in Ansible (yet), and we haven't decided how to proceed with such removals yet.

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added affects_2.10 feature This issue/PR relates to a feature request module module module_utils module_utils needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR needs_triage new_contributor Help guide this first time contributor plugins plugin (any type) remote_management tests tests labels Sep 23, 2020
@rajeevarakkal
Copy link
Contributor Author

Adding negative review to prevent premature merge. dellemc.openmanage is not included in Ansible (yet), and we haven't decided how to proceed with such removals yet.

Hi @felixfontein , are you referring to this PR ansible/ansible#71875 completion?

@felixfontein
Copy link
Collaborator

@rajeevarakkal no, I'm referring to the 'move content between collections where the target collection is not part of Ansible' policy which hasn't been decided yet.

Also, I don't think that PR in ansible/ansible will get merged, since ansible_builtin_runtime.yml is essentially frozen.

@felixfontein
Copy link
Collaborator

See ansible/community#539 (comment), "to discuss: what happens when the target collection is (not yet) contained in Ansible?".

@rajeevarakkal
Copy link
Contributor Author

@felixfontein , i didn't get exactly target collection not contained in ansible, yes this dellemc.openmanage collections are not ansible certified, is that what you mean?

@felixfontein
Copy link
Collaborator

No, I mean Ansible itself (https://pypi.org/project/ansible/) - it was formerly called ACD (Ansible Community Distribution), maybe that name is more familiar to you. The collections included in Ansible 2.10 are listed here: https://github.com/ansible-community/ansible-build-data/blob/main/2.10/ansible.in

@rajeevarakkal
Copy link
Contributor Author

@felixfontein , Thanks, will go through this once.

@gundalow
Copy link
Contributor

meta/runtime.yml needs updating for every plugin removed. An entry needs adding to point to the new collection.
.github/BOTMETA.yml also needs updating to remove the entries about these files and directories.

@rajeevarakkal
Copy link
Contributor Author

rajeevarakkal commented Sep 30, 2020

@gundalow , The three contributed modules from dellemc -ie remote_managment/dellemc are now we are able to publish as part of dellemc.openmanage collections in the ansible galaxy space.

There been a request/communicaton earlier that community.general is temporary space and teams need to host their collections in to Galaxy space or other repository.

Since we have our collections available at dellemc.openmanage, what should we do, what next?

@thedoubl3j
Copy link

@rajeevarakkal you still need to update the files @gundalow referenced before this can get merged

@ansibullbot ansibullbot added stale_ci CI is older than 7 days, rerun before merging unit tests/unit labels Oct 19, 2020
@felixfontein
Copy link
Collaborator

There seems to be no modules called dellemc_idrac and ome in the new collection, but you remove these here. Have they been renamed? And are the other modules compatible to their verisons in the DellEMC collection?

@felixfontein
Copy link
Collaborator

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

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

@felixfontein
Copy link
Collaborator

@rajeevarakkal could you please update/adjust this PR in a similar manner to e53f153 ? The removal would happen in community.general 3.0.0. Once this PR is in a good shape and ansible-community/ansible-build-data#58 has been merged, we can merge it and add an announcement to the next community.general 2.x.y release announcing this removal. (And of course update ansible/ansible#73046.)

@ansibullbot ansibullbot added community_review and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jan 28, 2021
@ansibullbot ansibullbot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review stale_ci CI is older than 7 days, rerun before merging labels Jan 29, 2021
@felixfontein
Copy link
Collaborator

@rajeevarakkal the PR now has conflicts, could you please rebase it?

@rajeevarakkal
Copy link
Contributor Author

@rajeevarakkal the PR now has conflicts, could you please rebase it?

on it. Resolving the same

@ansibullbot
Copy link
Collaborator

@rajeevarakkal this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibullbot
Copy link
Collaborator

@rajeevarakkal This PR was evaluated as a potentially problematic PR for the following reasons:

  • More than 50 changed files.

Such PR can only be merged by human. Contact a Core team member to review this PR on IRC: #ansible-devel on irc.freenode.net

click here for bot help

@ansibullbot ansibullbot added the merge_commit This PR contains at least one merge commit. Please resolve! label Jan 29, 2021
@felixfontein
Copy link
Collaborator

@rajeevarakkal could you please do a proper rebase to remove the merge commits, and remove all changes from this PR that do not belong in it (like the changes to .azure-pipelines/ and test/utils/)? Thanks.

@rajeevarakkal
Copy link
Contributor Author

rajeevarakkal commented Jan 29, 2021

uld you please do a proper rebase to remove the merge commits,

@felixfontein Looks like rebase is running into chained conflicts. I will go ahead nd create a fresh PR and provide the link here. Is that fine?

@felixfontein
Copy link
Collaborator

@rajeevarakkal that's fine as well!

@rajeevarakkal
Copy link
Contributor Author

@rajeevarakkal that's fine as well!

@felixfontein here is the new PR #1699

@rajeevarakkal
Copy link
Contributor Author

New PR created due to multiple conflict here - #1699

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This issue/PR relates to a feature request has_issue merge_commit This PR contains at least one merge commit. Please resolve! module_utils module_utils module module needs_maintainer needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_repo 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) remote_management tests tests unit tests/unit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants