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

Drop deprecated attribute 'Host#address' #15511

Merged
merged 1 commit into from
Jul 7, 2017

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Jul 5, 2017

Our TravisCI tests are generating deprecation warnings:

It was introduced by #14138

This removes them.

DEPRECATION WARNING:
address is deprecated and will be removed from ManageIQ G-release (hostname)
(called from block in ems_host_list at
/home/travis/build/ManageIQ/manageiq/app/models/vm_or_template.rb:792)

DEPRECATION WARNING:
address is deprecated and will be removed from ManageIQ G-release (hostname)
(called from new at
/home/travis/build/ManageIQ/manageiq/app/models/mixins/vim_connect_mixin.rb:22)

/cc @juliancheal Is this the solution you had in mind?
/cc @abellotti Does this affect the API (thinking no)
/cc @agrare Does the fix to vim_connect_mixin.rb look right to you? Or just drop the || part?

@kbrock kbrock added the core label Jul 5, 2017
@kbrock kbrock requested review from juliancheal and abellotti July 5, 2017 16:57
@agrare
Copy link
Member

agrare commented Jul 5, 2017

@kbrock definitely still need the || since most callers don't pass options (e.g.: remote_console_vmrc_acquire_ticket and find_vm_create_events)
The change to vim_connect_mixin looks good, do you need the self. though?

@kbrock kbrock force-pushed the deprecation_address branch from 6de446d to 501f663 Compare July 7, 2017 16:57
@agrare
Copy link
Member

agrare commented Jul 7, 2017

@kbrock yes a hostname will work, options[:ip] is badly named, any IP or dns name will work.

@miq-bot
Copy link
Member

miq-bot commented Jul 7, 2017

Checked commit kbrock@501f663 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

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

👍

@agrare agrare merged commit 2a2439a into ManageIQ:master Jul 7, 2017
@agrare agrare added this to the Sprint 64 Ending Jul 10, 2017 milestone Jul 7, 2017
@kbrock kbrock deleted the deprecation_address branch July 7, 2017 20:44
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