search

ManageIQ / manageiq-providers-hawkular

ManageIQ plugin for the Hawkular provider
8 stars 33 forks source link

Add i18n string placeholders test #109

Closed mzazrivec closed 6 years ago

mzazrivec commented 6 years ago

This is just invocation of standard i18n placeholder test, included from main ManageIQ repository. It's the same thing that we do in ui-classic to test that the provided translations honor placeholders in the original (English) strings.

miq-bot commented 6 years ago

Checked commit https://github.com/mzazrivec/manageiq-providers-hawkular/commit/30d713f8adf01ca182e718b44acb2eff08b71879 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0 1 file checked, 0 offenses detected Everything looks fine. :cookie:

abonas commented 6 years ago

@mzazrivec can you please explain why is this test needed in this repo? which code/placeholders should it test here? (the repo is server side focused) thanks

mzazrivec commented 6 years ago

I don't quite understand what 'server side focused repo' means.

Though what this test (shared spec) does is that it tests that the placeholders in gettext strings are preserved in translations. That means that for example if you have English string:

My name is %{name}

its translation into Spanish has to preserve the placeholder:

My nombre is %{name}

In the past, translators would make mistakes during translation: they would skip the placeholder or translate the string as:

My nombre is %{nombre}

or:

My nombre is {nombre}.

Either of these mistakes would cause problems in our application once it's switched to non-English locale.

We already have (run) this test in core manageiq as well as ui-classic and it needs to be included in all ManageIQ plugins (engines) that have thei rown gettext catalog.

abonas commented 6 years ago

We already have (run) this test in core manageiq as well as ui-classic and it needs to be included in all ManageIQ plugins (engines) that have thei rown gettext catalog.

this is the part that I am not fully understanding - where in this repo there is the gettext catalog? aren't all translations residing in miq ui classic?

mzazrivec commented 6 years ago

There's a shared spec (the test) in core manageiq and there's a gettext catalog in this repo under locale/ (every plugin has to have its own catalog). This PR just includes the shared spec, i.e. this PR causes that shared spec to be run for the gettext catalog in this repo.

karelhala commented 6 years ago

@mzazrivec so to understand this correctly, if we were to have some custom texts we should also update our locale/.

mzazrivec commented 6 years ago

@karelhala If you were to have some custom texts? I'm not quite sure what that means.

This repo contains gettext strings, so it has to have its own gettext catalog. This PR adds (or well, includes) a spec to test the translated catalog.

If you change the string inside a gettext call: _("some string...") or N_('some string ...') you don't have to update the catalog under locale/. That update only needs to be done when you want to upload the catalog to Zanata for translation.