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

Use the kubernetes options for Openshift #32

Merged

Conversation

enoodle
Copy link

@enoodle enoodle commented Jul 10, 2017

Using ManageIQ/manageiq-providers-kubernetes#45 for Openshift provider options.

This is required for using the options field with the Openshift provider as it adds the static description of the options (That is then being served through the API to the UI to present to the user when editing the options)

@enoodle enoodle changed the title Use the kubernetes options for Openshift [WIP] Use the kubernetes options for Openshift Jul 10, 2017
@miq-bot miq-bot added the wip label Jul 11, 2017
@enoodle enoodle changed the title [WIP] Use the kubernetes options for Openshift Use the kubernetes options for Openshift Aug 2, 2017
@miq-bot miq-bot removed the wip label Aug 2, 2017
Copy link

@moolitayer moolitayer left a comment

Choose a reason for hiding this comment

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

LGTM 👍
Needs to be merged after ManageIQ/manageiq-providers-kubernetes#

@enoodle enoodle closed this Aug 7, 2017
@enoodle enoodle reopened this Aug 7, 2017
@enoodle enoodle force-pushed the use_new_options_for_provider_settings branch from d49cbc2 to 322478a Compare August 8, 2017 08:39
@miq-bot
Copy link
Member

miq-bot commented Aug 8, 2017

This pull request is not mergeable. Please rebase and repush.

@enoodle enoodle force-pushed the use_new_options_for_provider_settings branch from 322478a to 8c222a6 Compare August 8, 2017 09:00
@enoodle enoodle force-pushed the use_new_options_for_provider_settings branch from 8c222a6 to cd40ce9 Compare August 10, 2017 14:29
@enoodle
Copy link
Author

enoodle commented Aug 10, 2017

ping @moolitayer

@miq-bot
Copy link
Member

miq-bot commented Aug 10, 2017

Checked commit enoodle@cd40ce9 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

@moolitayer
Copy link

@enoodle we have a minimal two reviewer per PR policy. Please get another review.

@enoodle
Copy link
Author

enoodle commented Aug 13, 2017

@yaacov @cben PTAL 😸

@@ -0,0 +1,4 @@
module ManageIQ::Providers::Openshift::ContainerManager::Options
extend ActiveSupport::Concern
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 without this, the ClassMethods would be available as class methods on this module, but not on Openshift::ContainerManager that includes it. 😭 😆 oh well ruby is weird sometimes...

Copy link
Member

@yaacov yaacov left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@moolitayer moolitayer merged commit 5f37b85 into ManageIQ:master Aug 13, 2017
@moolitayer moolitayer added this to the Sprint 67 Ending Aug 21, 2017 milestone Aug 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants