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 unsupported_reason_add and supports_event? #875

Merged
merged 2 commits into from
Mar 4, 2024

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Feb 27, 2024

  • Deprecating unsupported_reason_add. Returning the string instead
  • update to no longer use supports_events?

part of:

@kbrock
Copy link
Member Author

kbrock commented Feb 27, 2024

update:

  • fixed indentation
  • fixed paren in resize
  • rubocop [], unless else
  • dropped supports_events? => supports(:events) { ems.unsupported_reason }

update:

  • rubocop [], exclude?

@kbrock kbrock force-pushed the unsupports_supports branch 2 times, most recently from aa2d119 to ee4b53a Compare February 29, 2024 14:40
@kbrock
Copy link
Member Author

kbrock commented Feb 29, 2024

update:

  • convert snapshot remove_snapshot_by_description / revert_to_snapshot to supports_not
  • for cloud volume operations, convert return "" if ; return "" if to basic if ems "" ; else ""

@miq-bot
Copy link
Member

miq-bot commented Feb 29, 2024

Checked commits kbrock/manageiq-providers-openstack@b2440ed~...137c279 with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
26 files checked, 0 offenses detected
Everything looks fine. 🍪

Comment on lines +12 to +18
supports :events do
if parent_manager
parent_manager.unsupported_reason(:events)
else
_('no parent_manager to ems')
end
end
Copy link
Member Author

@kbrock kbrock Mar 1, 2024

Choose a reason for hiding this comment

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

@agrare:

This is a change.
before, if there is no parent_manager, then it will be good
after: if there is no parent manager, then it will bad

sound right to you?
(I obviously think so, but wanted to make this change clear)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I don't think this manager could possibly work without a parent ems due to the delegated authentication so even allow_nil below probably doesn't make sense so I'm good with this.

@agrare agrare merged commit e6e32c0 into ManageIQ:master Mar 4, 2024
3 checks passed
@kbrock kbrock deleted the unsupports_supports branch March 5, 2024 15:15
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