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

Convert host operations to supports #804

Merged
merged 2 commits into from
May 27, 2022

Conversation

agrare
Copy link
Member

@agrare agrare commented May 5, 2022


def validate_active_with_power_state(feature, expected_power_state)
return unsupported_reason_add(feature, _("The Host is not connected to an active Provider")) unless has_active_ems?
return unsupported_reason_add(feature, _("The host is not powered '#{expected_power_state}'")) unless expected_power_state == power_state
Copy link
Member

Choose a reason for hiding this comment

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

"This host is not powered 'maintenance'" is going to read weird. Not sure how else to do this though.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree but this is the current string, trying to not move and modify in the same change

supports :standby do
validate_active_with_power_state(:standby, "on")
end
supports :enter_maint_mode do
Copy link
Member

Choose a reason for hiding this comment

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

any reason to not spell it out?

Suggested change
supports :enter_maint_mode do
supports :enter_maintenance_mode do

Copy link
Member Author

Choose a reason for hiding this comment

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

@agrare agrare changed the title Convert host operations to supports [WIP] Convert host operations to supports May 5, 2022
Comment on lines 26 to 37
supports :vmotion do
validate_active_with_power_state(:vmotion, "on")
end
Copy link
Member Author

Choose a reason for hiding this comment

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

This one is a little weird, it really means "am I able to ask if vmotion is enabled" not actually checking if it is enabled might want to do this differently

@miq-bot miq-bot added the wip label May 5, 2022
@agrare agrare force-pushed the convert_host_operations_to_supports branch 2 times, most recently from 76d1f1f to 6810b9e Compare May 10, 2022 21:43
@agrare agrare changed the title [WIP] Convert host operations to supports Convert host operations to supports May 11, 2022
@Fryguy Fryguy added refactoring and removed wip labels May 11, 2022
@agrare agrare force-pushed the convert_host_operations_to_supports branch from 6810b9e to 23b0ab4 Compare May 26, 2022 21:49
@miq-bot
Copy link
Member

miq-bot commented May 26, 2022

Checked commits agrare/manageiq-providers-vmware@26c18e4~...23b0ab4 with ruby 2.6.9, rubocop 1.19.1, haml-lint 0.35.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. ⭐

@kbrock kbrock merged commit 95ac377 into ManageIQ:master May 27, 2022
@agrare agrare deleted the convert_host_operations_to_supports branch May 27, 2022 13:41
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.

4 participants