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.

Sign up for GitHub

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

Jump to bottom

Remove and redirect oc module. #866

Merged
merged 5 commits into from Dec 1, 2020
Merged

Remove and redirect oc module. #866

merged 5 commits into from Dec 1, 2020

Conversation

geerlingguy
Copy link
Contributor

@geerlingguy geerlingguy commented Sep 4, 2020 •
edited

SUMMARY

This pull request is to remove the oc connection plugin and redirect it to the community.okd module. We missed moving this module pre-Ansible 2.10, so we will do the redirect for now until the 2.0.0 of this collection comes out.

Related: openshift/community.okd#19

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

oc

@geerlingguy geerlingguy marked this pull request as ready for review September 4, 2020 21:11
@geerlingguy geerlingguy mentioned this pull request Sep 4, 2020
Closed
3 tasks
@ansibullbot
Copy link
Collaborator

@geerlingguy: Greetings! Thanks for taking the time to open this pullrequest. In order for the community to handle your pullrequest effectively, we need a bit more information.

Here are the items we could not find in your description:

  • issue type

Please set the description of this pullrequest with this template:
https://raw.githubusercontent.com/ansible/ansible/devel/.github/PULL_REQUEST_TEMPLATE.md

click here for bot help

@ansibullbot
Copy link
Collaborator

cc @chouseknecht @fabianvf @flaper87 @maxamillion
click here for bot help

@ansibullbot ansibullbot added affects_2.10 connection connection plugin has_issue needs_info This issue requires further information. Please answer any outstanding questions needs_template This issue/PR has an incomplete description. Please fill in the proposed template correctly needs_triage plugins plugin (any type) community_review feature This issue/PR relates to a feature request and removed needs_info This issue requires further information. Please answer any outstanding questions needs_template This issue/PR has an incomplete description. Please fill in the proposed template correctly labels Sep 4, 2020
felixfontein
felixfontein requested changes Sep 4, 2020
View reviewed changes
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.

Placing a negative review to avoid premature merging. This needs to be discussed in the community meeting first.

In general, since this removes functionality, it is a breaking change. (Redirects do not work for Ansible 2.9 which we support.) That's why IMO this has to wait for version 2.0.0 of the collection.

@ansibullbot ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review labels Sep 4, 2020
@felixfontein felixfontein mentioned this pull request Sep 4, 2020
Closed
@geerlingguy
Copy link
Contributor Author

@felixfontein thanks; honestly I don't know the exact procedure for this since we are moving out a plug-in that's already in 2.10...

@felixfontein
Copy link
Collaborator

@geerlingguy that's probably because we haven't defined that procedure yet ;) I've put it up on the agenda again (with this as an explicit example).

@felixfontein felixfontein mentioned this pull request Oct 15, 2020
Closed
21 tasks
@geerlingguy
Copy link
Contributor Author

Rebased.

@felixfontein felixfontein mentioned this pull request Nov 19, 2020
Closed
@aminvakil
Copy link
Contributor

Needs a rebase again.

@ansibullbot ansibullbot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Nov 26, 2020
@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.

@ansibullbot ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Nov 27, 2020
@felixfontein
Copy link
Collaborator

community.okd has been included in Ansible 2.10, so this can now be merged. Once it's rebased and updated :)

@geerlingguy
Copy link
Contributor Author

@felixfontein - I'll work on that in a bit!

@ansibullbot ansibullbot removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci CI is older than 7 days, rerun before merging labels Dec 1, 2020
felixfontein
felixfontein reviewed Dec 1, 2020
View reviewed changes
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.

Besides this detail, looks great!

changelogs/fragments/oc-migration-removal.yml Outdated Show resolved Hide resolved
@geerlingguy @felixfontein
Update changelogs/fragments/oc-migration-removal.yml
b586458 
Co-authored-by: Felix Fontein <felix@fontein.de>
@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 Dec 1, 2020
@felixfontein felixfontein added the breaking_change This PR contains a breaking change that MUST NOT be backported label Dec 1, 2020
@felixfontein
Copy link
Collaborator

The CI failures are unrelated.

@felixfontein felixfontein merged commit 1d8530a into ansible-collections:main Dec 1, 2020
@felixfontein
Copy link
Collaborator

@geerlingguy thanks for this PR!
@aminvakil thanks for reviewing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@felixfontein felixfontein felixfontein approved these changes

Assignees
No one assigned
Labels
breaking_change This PR contains a breaking change that MUST NOT be backported community_review connection connection plugin feature This issue/PR relates to a feature request has_issue plugins plugin (any type)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@geerlingguy @ansibullbot @felixfontein @aminvakil