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

Revision history for this message
Roman Shchekin (mrqtros) wrote :

Yep, I think we must use "strong" realization rather than a liitle bit "naive" =)

P.S. David, could your please have a look at my merge proposal? =)
I can't provide you with url, but my branch is named stable-trunk-prepare2 as you may remember.
Testing fails, I don't know why :(

22.07.13 10:23 David Planella написал(а):

Al 21/07/13 21:17, En/na Dariusz Gadomski ha escrit:
> Hi,
>
> Thanks for you opinion. Could you please explain (or provide a link to any docs) how to improve it? I have only experience with pure (no gettext) Qt/QML apps for MeeGo utilizing qsTr in both: *.qml and *.js files. I did not have any clue how to implement i18n in *.js, that's why I pass i18n object there from QML.
>
> In case of MeeGo using an array of strings along with qsTr (like the in line 463 in the diff below) worked fine for all languages (all the culture-specific stuff was done by translators: e.g. inserting Right-To-Left characters etc.).
>
> Do you know any examples of i18n usage in *.js files?
>
> Thanks,
> Dariusz
>

Also, I've just noticed that you're reimplementing a relative time
calculation that is already done in the Calculator app.

It might be worth looking at using just one way of calculating this.
Looking at both at a glance, yours looks simpler and cleaner, but the
calculator one handles translations and plurals for all languages.

Do you think you could have a look at unifying the two functions?

Cheers,
David.

--
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