-
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
Added new features for the Ansible UI move to the Automation tab #13526
Added new features for the Ansible UI move to the Automation tab #13526
Conversation
@miq-bot add_label ui, enhancement, wip |
9e90f68
to
f6df095
Compare
@dclarizio - please review |
@miq-bot remove_label wip |
- :name: Ansible Tower | ||
:description: Everything under Ansible Tower | ||
:feature_type: node | ||
:identifier: ansible_tower |
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.
I am really against us hardcoding this as ansible_tower. It should be reflected as AutomationManager, or, perhaps ExternalAutomationManager. The UI should never code to a specific provider but instead to the abstraction layer above it. cc @dclarizio @chessbyte
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.
@Fryguy - I agree - I had Automation Manager first, then discussed with @dclarizio and changed it to ansible_tower. I can change this to automation manager rather than external, and we can name the internal something else.
b3e7207
to
a4ac3f1
Compare
@Fryguy - updated |
- :name: Configured Systems | ||
:description: Everything under Configured Systems accordion | ||
:feature_type: node | ||
:identifier: automation_manager_cs_filter_accord |
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.
@lgalis Should this be abbreviated to _cs_
? I'm asking because
- the ones below, like
automation_manager_configured_system_control
are not abbreviated, and I feel it should be consistent. - Abbreviation of
cs
forconfigured_system
can be confused withconfiguration_script
. Down below, configuration_script is spelled out (for view:automation_manager_configuration_script_control
)
- :name: Create Service Dialog | ||
:description: Create Service Dialog | ||
:feature_type: admin | ||
:identifier: automation_manager_configscript_service_dialog |
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.
and this one is different in another way
- :name: View | ||
:description: View Configured Systems | ||
:feature_type: view | ||
:identifier: automation_manager_cs_filter_accord_view |
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.
One more _cs_
Rename feature to automation_manager
a4ac3f1
to
9a674bc
Compare
Checked commits lgalis/manageiq@7afe4b6~...33ae947 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
@Fryguy - please review |
Added new features for the Ansible UI move to the Automation tab
Links [Optional]
https://www.pivotaltracker.com/n/projects/1937457/stories/137618573
This PR is required for ManageIQ/manageiq-ui-classic#170