Code review comment for lp:~dgadomski/ubuntu-rssreader-app/shorts-pubdate-formatting

Revision history for this message
David Planella (dpm) wrote :

On Mon, Jul 22, 2013 at 9:05 AM, Dariusz Gadomski <email address hidden>wrote:

> I would love to improve my proposal, but I still miss some info.
>
> In the calculator app I can see the internationalization implemented in
> Storage.qml file as part of the Storage component. I wanted to have it
> isolated in a *.js library in rssreader. I was not able to find out how to
> use i18n object properly in *.js files, that is why I pass 'i18n' object
> form QML to JS and use it there.
>
>
I think what you're trying to do makes sense, only that I've never used the
i18n functions outside of QML files. You might want to try what Joey is
mentioning in his last comment.

> David: do you think it will not work in terms of
> internationalization/plurals? I am refering to line 463 of the diff below:
> return i18n.tr(FORMAT_LIMITS[i].formats[0],
> FORMAT_LIMITS[i].formats[1], val).arg(val)
> Will the translation tool catch up with such a dynamic call to i18n.tr?
> Please note that I provide all the information necessary for plurals
> handling there.
>
>
That's correct, I believe that'd work at runtime. However, what will not
work there are the tools that extract the translations from the code to
expose them to translators. Being more specific, I don't think xgettext
would work here, as it expects the translatable strings to be wrapped in
i18n.tr() calls.

Also note that while we want to start marking all strings for translation,
the RSS reader has not yet been set up for internationalization (which we
need to do as soon as we land some of the current big changes). I'm
mentioning this because if you want to test i18n, it might be necessary to
set it up first.

> If this is not supposed to work in *.js files I can prepare a QML
> component implementing this behaviour and reuse it in both: the calculator
> and the rssreader.
>
>
Sounds like a good idea too. Let us know how it goes.

> Cheers,
> Dariusz
> --
>
> https://code.launchpad.net/~dgadomski/ubuntu-rssreader-app/shorts-pubdate-formatting/+merge/176051
> Your team Ubuntu RSS Feed Reader Developers is requested to review the
> proposed merge of
> lp:~dgadomski/ubuntu-rssreader-app/shorts-pubdate-formatting into
> lp:ubuntu-rssreader-app.
>

« Back to merge proposal