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

Set minimum vSphere API version for WebMKS consoles #2538

Merged
merged 2 commits into from
Dec 18, 2017

Conversation

bmclaughlin
Copy link
Contributor

WebMKS is not available when a VM is using a vSphere API less than version 6.

@miq-bot add_labels bug, compute/infrastructure, fine/yes

https://bugzilla.redhat.com/show_bug.cgi?id=1483936

@miq-bot
Copy link
Member

miq-bot commented Nov 10, 2017

This pull request is not mergeable. Please rebase and repush.

@@ -7,8 +7,18 @@ def visible?
end

def disabled?
@error_message = _("The web-based WebMKS console is not available because the required libraries aren't installed") unless webmks_assets_provided?
@error_message = _('The web-based WebMKS console is not available because the VM is not powered on') unless on?
canned_msg = _('The web-based WebMKS console is not available because')
Copy link
Member

Choose a reason for hiding this comment

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

Please, don't compose senteses via string concatenation. This might not translate to other languates :-(

Replacing the "because" with ":" and starting with a capital letter. Might be a bette choice.

ping @mzazrivec

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct. The message needs to stay together. I get complaints regarding this from translators.

@martinpovolny
Copy link
Member

Also changing the strings in in the Fine branch is likely to produce missing translations. I doubt the catalogs are passed throught the translator team for Fine.

@mzazrivec ?

@mzazrivec
Copy link
Contributor

Yes, should the fix land in some of the older branches, we need to avoid changing the strings
if possible.

@martinpovolny
Copy link
Member

@bmclaughlin : please, reformat this PR is such a way that the strings do not change. Thx!

@miq-bot
Copy link
Member

miq-bot commented Dec 4, 2017

Checked commits bmclaughlin/manageiq-ui-classic@35eea08~...77f81cc with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

@bmclaughlin
Copy link
Contributor Author

@martinpovolny, @mzazrivec Reverted the String changes. Not sure if the CC issue needs to be addressed? Thoughts?

@martinpovolny
Copy link
Member

@mzazrivec : can we merge this one?

@mzazrivec mzazrivec added this to the Sprint 76 Ending Jan 1, 2018 milestone Dec 18, 2017
@mzazrivec mzazrivec merged commit 499d120 into ManageIQ:master Dec 18, 2017
@bmclaughlin bmclaughlin deleted the set-min-api-version-for-webmks branch December 18, 2017 14:17
simaishi pushed a commit that referenced this pull request Dec 18, 2017
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 2d139ed8f665e4e8e52d7334ed41128bed25fd43
Author: Milan Zázrivec <mzazrivec@redhat.com>
Date:   Mon Dec 18 12:37:26 2017 +0100

    Merge pull request #2538 from bmclaughlin/set-min-api-version-for-webmks
    
    Set minimum vSphere API version for WebMKS consoles
    (cherry picked from commit 499d12058e3c54d91b7a60ade5f8a4851b81a634)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1527106

simaishi pushed a commit that referenced this pull request Jan 4, 2018
@simaishi
Copy link
Contributor

simaishi commented Jan 4, 2018

Fine backport details:

commit 0a6562b7e42dd4ad1f8f5ee0a33a4100ae40ae3a
Author: Milan Zázrivec <mzazrivec@redhat.com>
Date:   Mon Dec 18 12:37:26 2017 +0100

    Merge pull request #2538 from bmclaughlin/set-min-api-version-for-webmks
    
    Set minimum vSphere API version for WebMKS consoles
    (cherry picked from commit 499d12058e3c54d91b7a60ade5f8a4851b81a634)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1531280

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.

6 participants