ubuntu-touch-coreapps-reviewers team mailing list archive

[David Planella <david.planella@xxxxxxxxxx>]

Review: Approve

Looks good to me, just added two notes.

Diff comments:

> === modified file 'app/alarm/AlarmSettingsPage.qml'
> --- app/alarm/AlarmSettingsPage.qml	2014-10-16 19:03:51 +0000
> +++ app/alarm/AlarmSettingsPage.qml	2015-03-12 23:25:02 +0000
> @@ -50,10 +50,10 @@
>          Component.onCompleted: initialise()
>  
>          function initialise() {
> -            durationModel.append({ "duration": 10, "text": i18n.tr("%1 minutes").arg(10) })
> -            durationModel.append({ "duration": 20, "text": i18n.tr("%1 minutes").arg(20) })
> -            durationModel.append({ "duration": 30, "text": i18n.tr("%1 minutes").arg(30) })
> -            durationModel.append({ "duration": 60, "text": i18n.tr("%1 minutes").arg(60) })
> +            durationModel.append({ "duration": 10, "text": i18n.tr("%1 minute", "%1 minutes", 10).arg(10) })
> +            durationModel.append({ "duration": 20, "text": i18n.tr("%1 minute", "%1 minutes", 20).arg(20) })
> +            durationModel.append({ "duration": 30, "text": i18n.tr("%1 minute", "%1 minutes", 30).arg(30) })
> +            durationModel.append({ "duration": 60, "text": i18n.tr("%1 minute", "%1 minutes", 60).arg(60) })
>          }

It's a bit of a strange case, as the string will always be plural, but I think it's better like this, as this way we'll leave the plural handling of gettext manage which plural form to use for those languages with more complex plural requirements.

>      }
>  
> @@ -62,10 +62,10 @@
>          Component.onCompleted: initialise()
>  
>          function initialise() {
> -            snoozeModel.append({ "duration": 2, "text": i18n.tr("%1 minutes").arg(2) })
> -            snoozeModel.append({ "duration": 4, "text": i18n.tr("%1 minutes").arg(4) })
> -            snoozeModel.append({ "duration": 5, "text": i18n.tr("%1 minutes").arg(5) })
> -            snoozeModel.append({ "duration": 10, "text": i18n.tr("%1 minutes").arg(10) })
> +            snoozeModel.append({ "duration": 2, "text": i18n.tr("%1 minute", "%1 minutes", 2).arg(2) })
> +            snoozeModel.append({ "duration": 4, "text": i18n.tr("%1 minute", "%1 minutes", 4).arg(4) })
> +            snoozeModel.append({ "duration": 5, "text": i18n.tr("%1 minute", "%1 minutes", 5).arg(5) })
> +            snoozeModel.append({ "duration": 10, "text": i18n.tr("%1 minute", "%1 minutes", 10).arg(10) })
>          }

Same as the comment above

>      }
>  
> @@ -143,7 +143,7 @@
>                          SubtitledListItem {
>                              id: _header
>                              text: i18n.tr("Silence after")
> -                            subText: i18n.tr("%1 minutes").arg(alarmSettings.duration)
> +                            subText: i18n.tr("%1 minute", "%1 minutes", alarmSettings.duration).arg(alarmSettings.duration)
>                              onClicked: _alarmDuration.expanded = true
>  
>                              Icon {
> @@ -212,7 +212,7 @@
>                          SubtitledListItem {
>                              id: _snoozeHeader
>                              text: i18n.tr("Snooze for")
> -                            subText: i18n.tr("%1 minutes").arg(alarmSettings.snoozeDuration)
> +                            subText: i18n.tr("%1 minute", "%1 minutes", alarmSettings.snoozeDuration).arg(alarmSettings.snoozeDuration)
>                              onClicked: _alarmSnooze.expanded = true
>  
>                              Icon {
> 
> === modified file 'debian/changelog'
> --- debian/changelog	2015-03-06 17:12:24 +0000
> +++ debian/changelog	2015-03-12 23:25:02 +0000
> @@ -10,7 +10,8 @@
>    * Fixed predefined cities and countries not being translatable in the timezone
>      selection dialog (LP: #1354466)
>    * Fixed empty state description not wrapping (LP: #1428165)
> -  * Fixed edit alarm crash issue in vivid (thanks to Zsombor)
> +  * Fixed edit alarm crash issue in vivid (thanks to Zsombor) (LP: #1429273)
> +  * Fixed strings not following gettext-style plural forms. (LP: #1431446)
>  
>    [Brendan Donegan]
>    * Fixed AP failure by waiting for the bottom edge tip visible property to be true
> 
> === modified file 'po/com.ubuntu.clock.pot'
> --- po/com.ubuntu.clock.pot	2015-03-06 17:11:42 +0000
> +++ po/com.ubuntu.clock.pot	2015-03-12 23:25:02 +0000
> @@ -8,7 +8,7 @@
>  msgstr ""
>  "Project-Id-Version: \n"
>  "Report-Msgid-Bugs-To: \n"
> -"POT-Creation-Date: 2015-03-06 17:09+0000\n"
> +"POT-Creation-Date: 2015-03-13 00:18+0100\n"
>  "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
>  "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
>  "Language-Team: LANGUAGE <LL@xxxxxx>\n"
> @@ -16,6 +16,7 @@
>  "MIME-Version: 1.0\n"
>  "Content-Type: text/plain; charset=UTF-8\n"
>  "Content-Transfer-Encoding: 8bit\n"
> +"Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\n"
>  
>  #: ../app/alarm/AlarmLabel.qml:30 ../app/alarm/AlarmLabel.qml:56
>  #: ../app/alarm/EditAlarmPage.qml:291
> @@ -71,8 +72,10 @@
>  #: ../app/alarm/AlarmSettingsPage.qml:146
>  #: ../app/alarm/AlarmSettingsPage.qml:215
>  #, qt-format
> -msgid "%1 minutes"
> -msgstr ""
> +msgid "%1 minute"
> +msgid_plural "%1 minutes"
> +msgstr[0] ""
> +msgstr[1] ""
>  
>  #: ../app/alarm/AlarmSettingsPage.qml:92
>  msgid "Alarm volume"
> @@ -1786,7 +1789,7 @@
>  msgstr ""
>  
>  #: ../backend/modules/Timezone/statictimezonemodel.cpp:306
> -msgid "Simferopol’"
> +msgid "Simferopol"
>  msgstr ""
>  
>  #: ../backend/modules/Timezone/statictimezonemodel.cpp:307
> 


-- 
https://code.launchpad.net/~nik90/ubuntu-clock-app/fix-translation-plural-forms/+merge/252838
Your team Ubuntu Clock Developers is subscribed to branch lp:ubuntu-clock-app.