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

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

Thanks!

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

Al 22/07/13 08:34, En/na Roman Shchekin ha escrit:
> 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 :(

Hi Roman,

I already commented on your other branch this morning.

Cheers,
David.

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