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

Add default_port needed for params_for_create #155

Merged

Conversation

agrare
Copy link
Member

@agrare agrare commented Nov 13, 2019

Use the params_for_create and verify_credential methods from ContainerManagerMixin in kubernetes, just need to define the default_port

Depends on: ManageIQ/manageiq-providers-kubernetes#341

ManageIQ/manageiq#18818

@miq-bot
Copy link
Member

miq-bot commented Nov 13, 2019

Checked commit agrare@58d6757 with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. ⭐

@gtanzillo gtanzillo added this to the Sprint 125 Ending Nov 25, 2019 milestone Nov 13, 2019
@gtanzillo gtanzillo merged commit a41c926 into ManageIQ:master Nov 13, 2019
@agrare agrare deleted the add_params_for_create_and_verify_credentials branch November 13, 2019 22:37
@cben
Copy link
Contributor

cben commented Nov 14, 2019

LGTM 👍

I remember the const DEFAULT_PORT resolution rules has bitten me in the past, I'm a bit itching to drop the const and replace all its uses with calling the class method :)

@agrare
Copy link
Member Author

agrare commented Nov 14, 2019

❤️ I agree I was quite surprised to find that my method in ContainerManagerMixin didn't pull in the constant from Openshift

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants