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

Add ems_infra_admin_ui feature #16403

Merged
merged 1 commit into from
Nov 14, 2017
Merged

Conversation

vojtechszocs
Copy link
Contributor

@vojtechszocs vojtechszocs commented Nov 6, 2017

This PR is part of the following series:

The ems_infra_admin_ui product feature represents Infrastructure Provider's capability to link to its own (provider-specific) administration UI.

This feature is currently associated with EvmRole-operator (just like ems_infra_refresh).

Inspired by @jhernand (see original PR here).

@vojtechszocs
Copy link
Contributor Author

cc @himdel @oourfali

@@ -86,6 +86,7 @@ module SupportsFeatureMixin
:discovery => 'Discovery of Managers for a Provider',
:evacuate => 'Evacuation',
:launch_cockpit => 'Launch Cockpit UI',
:admin_ui => 'Open Admin UI for a Provider',
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest to add oVirt somewhere in the description, and maybe also in the name of the feature.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just saw in other pull request that you are adding a supports mechanism to the provider. If that is going to be generic then you can disregard this comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I've consulted this with @himdel and decided to make this feature generic - each Infra Provider would have the option to support the admin_ui product feature.

@@ -6526,7 +6530,7 @@
- :name: Switch Language
:description: Display Language options
:feature_type: view
:identifier: sui_language
:identifier: sui_language
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is better if you don't do unrelated fixes in this pull request. If you think these should be fixed, do it in a separate pull request.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. My IDE automatically strips trailing whitespace. I'll revert those changes.

Signed-off-by: Vojtech Szocs <vojtech.szocs@gmail.com>
@miq-bot
Copy link
Member

miq-bot commented Nov 10, 2017

Checked commit vojtechszocs@9c34aed with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 1 offense detected

app/models/mixins/supports_feature_mixin.rb

  • ❗ - Line 89, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.

@agrare agrare merged commit 141cdac into ManageIQ:master Nov 14, 2017
@agrare agrare added this to the Sprint 74 Ending Nov 27, 2017 milestone Nov 14, 2017
@vojtechszocs
Copy link
Contributor Author

@simaishi we would like to backport this to 5.9 but I can't find a way how to add the gaprindashvili/yes label. Can you please assist?

@oourfali
Copy link

@miq-bot add_label gaprindashvili/yes

simaishi pushed a commit that referenced this pull request Nov 17, 2017
Add ems_infra_admin_ui feature
(cherry picked from commit 141cdac)
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit c7680af415e0535a8e316c1bb0b49db13c78f5f3
Author: Adam Grare <agrare@redhat.com>
Date:   Tue Nov 14 08:31:14 2017 -0500

    Merge pull request #16403 from vojtechszocs/link-ovirt-ui
    
    Add ems_infra_admin_ui feature
    (cherry picked from commit 141cdac427cdb755748db82a431ee217f944e09a)

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.

8 participants