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

Fix outdated branded strings for oVirt and OpenStack and fix broken docs link #51

Merged

Conversation

Thorben-D
Copy link
Contributor

While investigating how this plugin handles downstream branding, I noticed that some branded words are out of date.

  • "Red Hat Enterprise Virtualization" was renamed to "Red Hat Virtualization"
  • "RHEL Openstack Platform" was renamed to "Red Hat Openstack Platform"
  • The link to the docs for registering a host using the bootstrap script was changed from this to this with Satellite 6.11

@Thorben-D Thorben-D force-pushed the fixes/update_branded_strings branch from 10946db to 75aaf23 Compare July 28, 2024 16:01
@ShimShtein
Copy link
Contributor

Another rebase needed, sorry!

@Thorben-D Thorben-D force-pushed the fixes/update_branded_strings branch from 75aaf23 to 2fabc8d Compare July 31, 2024 07:36
Copy link
Contributor

@ShimShtein ShimShtein left a comment

Choose a reason for hiding this comment

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

LGTM. Any additional concerns?

@ShimShtein
Copy link
Contributor

@Thorben-D Please make sure the tests are green

Comment on lines 13 to 14
/\b[Pp]roxy [Hh][Tt][Tt][Pp]\b(?!-)/ => 'Proxy HTTP', # Workaround for French translation
/\b[Pp]roxies [Hh][Tt][Tt][Pp]\b(?!-)/ => 'Proxies HTTP', # Workaround for French translation
Copy link
Contributor

Choose a reason for hiding this comment

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

We still need those, otherwise in French Proxies HTTP will be branded as Capsules HTTP (which is wrong).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh right, I forgot French does this...
While we're at it, what about "non-latin alphabet" languages? I noticed that in, for example, Russian, "Create Smart Proxy" is translated to "Создать Смарт Прокси", which will definitely not be branded correctly.
Are there any plans on covering these languages? Otherwise, I'd check it out myself.

Copy link
Contributor

Choose a reason for hiding this comment

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

My assumption was (and I have actually outlined it in the redme) that we don't want to translate names.
Meaning that we should translate "Create Smart Proxy" to "Создать Smart Proxy".
I know that for Hebrew for example we do leave Latin names for such things. I have to admit I am not sure if it is considered an OK practice in other languages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I very much agree with the idea of not translating names. I checked the translation files and found the following:
Note: for most of these I had to rely on DeepL, so no promises...

Language Translation of "Smart Proxy" Translation into English
Catalan servidor intermediari intel·ligent smart proxy server
Czech none none
German none none
English (US) none none
English (UK) none none
Spanish proxy inteligente smart proxy
Italian none none
Japanese none none
Georgian ჭკვიანი პროქსი smart proxy
Korean 스마트 프록시 smart proxy
Polish smart proxy / inteligentnych proxy smart proxy
Portuguese (BR) smart proxy / proxy Inteligente smart proxy
Russian Смарт Прокси smart proxy
Chinese (ZN) 智能代机 smart proxy
Chinese (TW) 智慧型代理伺服器 smart proxy server

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm.. Interesting. I see in Russian and Korean it's just a transliteration (It's the same text, just in Cyrillic/Korean letters) for other languages here it's actually a translation: (Smart -> Intelligent). And for Georgian it's a mix - "Proxy" is transliterated, but "Smart" is translated.
Specifically for our issue, Polish and Portuguese may become problematic, since they would be branded as "Capsule Inteligente" for Portuguese.

Copy link
Contributor

Choose a reason for hiding this comment

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

I very much agree with the idea of not translating names. I checked the translation files and found the following:

I would expect all of them to be in the glossary:

https://app.transifex.com/foreman/foreman_2013-08-14/translate/#en/foreman_2013-08-14?q=text%3Asmart

Yet when I check some languages I don't see them.

lib/foreman_theme_satellite/branded_words.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@ShimShtein ShimShtein left a comment

Choose a reason for hiding this comment

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

A couple of small changes please.

lib/foreman_theme_satellite/branded_words.rb Outdated Show resolved Hide resolved
lib/foreman_theme_satellite/branded_words.rb Outdated Show resolved Hide resolved
/\bOpenStack\b(?!-)/ => 'RHEL OpenStack Platform',
/\bopenstack\b(?!-)/ => 'RHEL OpenStack Platform',
/\bOpenstack\b(?!-)/ => 'RHEL OpenStack Platform',
/\bProxy\b(?!-)/ => 'Capsule',
Copy link
Contributor

Choose a reason for hiding this comment

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

We still need those, while imprecise string are in the core.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I re-added a check for proxy/proxies. OpenStack branding is already covered by line 35.

- Red Hat Virtualization: Change from "RHEV" to "RHV"
- Red Hat Openstack Platform: Change from "RHEL Openstack Platform" to "Red Hat Openstack Platform"
- Fix link to docs for registering hosts using bootstrap script
- Simplify regular expressions
- Add test based on documentation strings
@ShimShtein ShimShtein merged commit f78f229 into RedHatSatellite:develop Aug 22, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants