-
Notifications
You must be signed in to change notification settings - Fork 898
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
Changed Feature Name from native to management_console #22474
base: master
Are you sure you want to change the base?
Changed Feature Name from native to management_console #22474
Conversation
@agrare can you make sure I'm not missing anything or changed something that's not necessary to change. |
d70a3ed
to
e85caa1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need supports_not :management_console
in ExtManagementSystem
We also need to change the supports feature for the supports feature from :native_console
to :management_console
in the providers which implement console_url for ExtManagementSystems.
@DavidResende0 we still need to update the other providers which implement this at the EMS level per #22474 (review) |
Are there other providers besides ManageIQ/manageiq-providers-ibm_power_hmc#136 and ManageIQ/manageiq-providers-ibm_cloud#451 that I'm missing? Also I'm I missing something in these prs? |
Yes, https://github.com/search?q=org%3AManageIQ+%22supports+%3Anative_console%22&type=code |
80ce8af
to
ca3126e
Compare
ca3126e
to
56897fa
Compare
@@ -278,6 +278,7 @@ | |||
- vm_infra_explorer | |||
- vm_html5_console | |||
- vm_vmrc_console | |||
- vm_management_console | |||
- vm_native_console |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we still want the vm_native_console
entries?
ugh. this probably requires a migration and everything.
^ ignore. this looks great
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes because vm_native_console is still a valid feature (ovirt/rhv vm console), it was being overloaded by something else (web browser details page)
56897fa
to
5c665f5
Compare
Checked commit DavidResende0@5c665f5 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint |
@DavidResende0 what is the status of this? Were we waiting on testing or other providers? |
I think these changes cover all the needed providers and everything seems to work on the one's I've tested but I haven't been able to test every provider. |
This pull request is not mergeable. Please rebase and repush. |
This pull request has been automatically marked as stale because it has not been updated for at least 3 months. If these changes are still valid, please remove the |
2 similar comments
This pull request has been automatically marked as stale because it has not been updated for at least 3 months. If these changes are still valid, please remove the |
This pull request has been automatically marked as stale because it has not been updated for at least 3 months. If these changes are still valid, please remove the |
Follow up to: ManageIQ/manageiq-ui-classic#8758
Changes the feature name for the IBM providers from
native_console
tomanagement_console
sincenative_console
is already used by the RHV provider. Also fixes a bug in the ui that was being caused by this overlap of feature names.Other PRs:
ui -> ManageIQ/manageiq-ui-classic#8762
Lenovo -> ManageIQ/manageiq-providers-lenovo#389
vmware -> ManageIQ/manageiq-providers-vmware#874
ibm_power_hmc -> ManageIQ/manageiq-providers-ibm_power_hmc#136
ibm_terraform -> ManageIQ/manageiq-providers-ibm_terraform#94
ibm_cloud -> ManageIQ/manageiq-providers-ibm_cloud#451
ibm_power_vc -> ManageIQ/manageiq-providers-ibm_power_vc#90