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 Instance (IBM) to en.yml #20800

Merged
merged 1 commit into from
Nov 12, 2020
Merged

Conversation

lpichler
Copy link
Contributor

Visible for example here:
Screenshot 2020-11-11 at 17 09 32

cc @mzazrivec @agrare

@Fryguy
Copy link
Member

Fryguy commented Nov 11, 2020

Would be nice to get Dictionary pluggable (#19440), if we choose to continue with the Dictionary. cc @agrare

or perhaps, in this particular case there should be a display_name method on the object itself (which I thought we had?)

@miq-bot
Copy link
Member

miq-bot commented Nov 11, 2020

Checked commit lpichler@f9b80d1 with ruby 2.6.3, rubocop 0.82.0, haml-lint 0.35.0, and yamllint
0 files checked, 0 offenses detected
Everything looks fine. 🍪

@lpichler
Copy link
Contributor Author

or perhaps, in this particular case there should be a display_name method on the object itself (which I thought we had?)

Yes I believe that we can:

entities = OpsController::Settings::LabelTagMapping::MAPPABLE_ENTITIES.to_a.reject{|x| x[1].model.nil? || x[1].prefix.nil? }

#  WITH ui_lookup - CURRENT
entities.map { |x| ui_lookup(:model=>x[1].model) }
# =>
[
    [ 0] "Instance (Amazon)",
    [ 1] "Instance (IBM)",
    [ 2] "Instance (OpenStack)",
    [ 3] "Instance (Microsoft Azure)",
    [ 4] "Virtual Machine (VMware)",
    [ 5] "Image (Amazon)",
    [ 6] "Container Project",
    [ 7] "Container Route",
    [ 8] "Container Node",
    [ 9] "Container Replicator",
    [10] "Container Service",
    [11] "Container Pod",
    [12] "Container Build"
]

# WITH display_name

entities .map { |x| x[1].model.safe_constantize.display_name }
# =>
[
    [ 0] "Instance (Amazon)",
    [ 1] "Instance (IBM)",
    [ 2] "Instance (OpenStack)",
    [ 3] "Instance (Microsoft Azure)",
    [ 4] "Virtual Machine (VMware)",
    [ 5] "Image (Amazon)",
    [ 6] "Container Project",
    [ 7] "Container Route",
    [ 8] "Container Node",
    [ 9] "Container Replicator",
    [10] "Container Service",
    [11] "Pod",
    [12] "Container Build"

I will include it in other PR after #20776

thanks @Fryguy

@mzazrivec
Copy link
Contributor

Would be nice to get Dictionary pluggable (#19440), if we choose to continue with the Dictionary. cc @agrare

or perhaps, in this particular case there should be a display_name method on the object itself (which I thought we had?)

Yes, we do have self.display_name() methods, which solve the pluggability problem. The only thing remaining is to switch Dictionary.gettext() from using en.yml to the above method. I am currently working on this: the switch itself is not a big problem, but it will cause quite a bit of CI failures in multiple repos, so therefore it's not complete yet.

@chessbyte chessbyte self-assigned this Nov 12, 2020
@chessbyte chessbyte merged commit ace7b47 into ManageIQ:master Nov 12, 2020
simaishi pushed a commit that referenced this pull request Nov 12, 2020
Add `Instance (IBM)` to en.yml

(cherry picked from commit ace7b47)
@simaishi
Copy link
Contributor

Kasparov backport details:

$ git log -1
commit 38be0998a03a474fe0cc17ba8b76372cecea2b08
Author: Oleg Barenboim <[email protected]>
Date:   Thu Nov 12 09:21:40 2020 -0500

    Merge pull request #20800 from lpichler/ibm_cloud_locale

    Add `Instance (IBM)` to en.yml

    (cherry picked from commit ace7b477e70c3819047768276fee6ad0900ebbc4)

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.

7 participants