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

Fix general CloudNetwork class_by_ems method #14392

Merged
merged 1 commit into from
Mar 27, 2017

Conversation

tzumainn
Copy link
Contributor

@tzumainn tzumainn commented Mar 17, 2017

Only OpenStack networks contain Public/Private versions, so class_by_ems will fail for other provider
cloud networks. This fix remediates that.

https://bugzilla.redhat.com/show_bug.cgi?id=1403152

@Ladas
Copy link
Contributor

Ladas commented Mar 21, 2017

looks great 👍

@@ -41,11 +41,7 @@ class CloudNetwork < ApplicationRecord

def self.class_by_ems(ext_management_system, external = false)
Copy link
Member

Choose a reason for hiding this comment

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

If external is now ignored here, can you change the parameter name here to _external?

@tzumainn tzumainn force-pushed the cloud-network-class-by-ems-fix branch from 8b9c852 to 0336d72 Compare March 24, 2017 14:20
@tzumainn
Copy link
Contributor Author

@blomquisg made the change you suggested, thanks!

@miq-bot
Copy link
Member

miq-bot commented Mar 24, 2017

Checked commit tzumainn@0336d72 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks good. ⭐

@blomquisg
Copy link
Member

@miq-bot merge when green

/cc @chrisarcand

@miq-bot
Copy link
Member

miq-bot commented Mar 24, 2017

@blomquisg Maybe I'd do it...if it were green... 😒👇

@blomquisg
Copy link
Member

@Fryguy miq-bot was clearly hacked

@tzumainn tzumainn closed this Mar 25, 2017
@tzumainn tzumainn reopened this Mar 25, 2017
@tzumainn tzumainn closed this Mar 25, 2017
@tzumainn tzumainn reopened this Mar 25, 2017
@agrare agrare merged commit 30144fa into ManageIQ:master Mar 27, 2017
@agrare agrare added this to the Sprint 57 Ending Mar 27, 2017 milestone Mar 27, 2017
@tzumainn
Copy link
Contributor Author

@miq-bot add_label euwe/yes

simaishi pushed a commit that referenced this pull request Apr 13, 2017
@simaishi
Copy link
Contributor

Euwe backport details:

$ git log -1
commit d934bc2672e6c74a3f41aa96ad27daa633679da4
Author: Adam Grare <[email protected]>
Date:   Mon Mar 27 11:58:37 2017 -0400

    Merge pull request #14392 from tzumainn/cloud-network-class-by-ems-fix
    
    Fix general CloudNetwork class_by_ems method
    (cherry picked from commit 30144fae7ce7f2c08995d89be22b376446da4e7d)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1437148

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.

8 participants