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

Replace MiqUUID with inline clean_guid method #588

Merged
merged 3 commits into from
Jun 8, 2020
Merged

Replace MiqUUID with inline clean_guid method #588

merged 3 commits into from
Jun 8, 2020

Conversation

djberg96
Copy link
Contributor

@djberg96 djberg96 commented Jun 6, 2020

Since the VMWare provider is the only provider that uses this code, let's move it directly into this code base, and out of gems-pending.

Part of the ManageIQ/manageiq-gems-pending#231 cleanup.

@djberg96 djberg96 requested review from agrare and Fryguy as code owners June 6, 2020 15:22
@agrare agrare self-assigned this Jun 7, 2020
@@ -28,7 +30,7 @@ def parse_virtual_machine_summary(vm_hash, props)
if summary_config
uuid = summary_config[:uuid]
unless uuid.blank?
vm_hash[:uid_ems] = MiqUUID.clean_guid(uuid) || uuid
vm_hash[:uid_ems] = clean_guid(uuid) || uuid
Copy link
Member

Choose a reason for hiding this comment

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

@djberg96 I think we can just do vm_hash[:uid_ems] = clean_guid(uuid) without the || uuid if the only two cases that method returns nil for is if the uuid is nil (in which case we'd get nil || nil) or if the string is empty which isn't a valid uid_ems anyway. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@agrare I agree, will update.

@miq-bot
Copy link
Member

miq-bot commented Jun 8, 2020

Checked commits https://github.com/djberg96/manageiq-providers-vmware/compare/4ddceb3b07c71ffdd1da263dba01d423cc962e8d~...7b4711382fc95d9cb7c5784b970bed6161c776f3 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
1 file checked, 2 offenses detected

app/models/manageiq/providers/vmware/infra_manager/inventory/parser/virtual_machine.rb

@agrare agrare merged commit 6c9269e into ManageIQ:master Jun 8, 2020
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.

3 participants