Code review comment for lp:~nik90/ubuntu-clock-app/make-alarmlabel-translatable

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

Looks good to me. However, I've noticed some updates in the .pot file that probably need to be fixed in a separate branch:

186 -msgid "%1 h %2 m %3"
187 +msgid "%1h %2m %3"
188 msgstr ""
189
190 #. TRANSLATORS: the first argument is hour, followed by the
191 #. translation for either 'behind' or 'ahead'
192 #: ../app/worldclock/UserWorldCityDelegate.qml:160
193 #, qt-format
194 -msgid "%1 h %2"
195 +msgid "%1h %2"
196 msgstr ""
197
198 #. TRANSLATORS: the first argument is minute, followed by the
199 #. translation for either 'behind' or 'ahead'
200 #: ../app/worldclock/UserWorldCityDelegate.qml:168
201 #, qt-format
202 -msgid "%1 m %2"
203 +msgid "%1m %2"

Numerical value and unit symbol are separated by a space (see [1]), which seems to have been reverted in a previous branch. This is not just about being pedantic: attaching letters to %1 arguments modifies the way the arguments behave, therefore it's always best to leave the %1 args on their own. As an example, if I try to translate this string in LP: "%1m %2" as "%1 m %2", gettext complains that the translated argument needs to be the same as the original, and does not allow me to submit the translation.

So approving it, but with caveats for this to be changed in a separate branch.

[1] http://en.wikipedia.org/wiki/ISO_31-0

review: Approve

« Back to merge proposal