Merge lp://qastaging/~dpm/reminders-app/update-translations into lp://qastaging/reminders-app

Proposed by David Planella
Status: Rejected
Rejected by: Francis Ginther
Proposed branch: lp://qastaging/~dpm/reminders-app/update-translations
Merge into: lp://qastaging/reminders-app
Diff against target: 302 lines (+99/-27)
10 files modified
.bzrignore (+1/-0)
po/CMakeLists.txt (+1/-0)
po/com.ubuntu.reminders.pot (+79/-10)
src/app/qml/components/NotebooksDelegate.qml (+1/-1)
src/app/qml/ui/EditNotePage.qml (+6/-5)
src/app/qml/ui/NotePage.qml (+3/-3)
src/app/qml/ui/NotebooksPage.qml (+3/-3)
src/app/qml/ui/NotesPage.qml (+2/-2)
src/app/qml/ui/RemindersPage.qml (+2/-2)
src/app/qml/ui/SearchNotesPage.qml (+1/-1)
To merge this branch: bzr merge lp://qastaging/~dpm/reminders-app/update-translations
Reviewer Review Type Date Requested Status
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Michael Zanetti (community) Approve
Review via email: mp+203030@code.qastaging.launchpad.net

Commit message

Update and fix translatable strings

Description of the change

As part of the core apps hack days, we want to ensure translations are in shape for contribution. This MP:

- Updates the .pot file to expose the new translatable strings in Launchpad
- Makes a few more strings translatable
- Makes the cmake rule to update the .pot file work (run it with `make com.ubuntu.reminders.pot`)
- Fixes a plural string
- Capitalizes some of the actions to make them consistent with the rest of the UI
- Adds a few icons to some of the actions

To post a comment you must log in.
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Michael Zanetti (mzanetti) wrote :

Nice branch. Thanks.

One comment:
> - Makes the cmake rule to update the .pot file work (run it with `make com.ubuntu.reminders.pot`)

shouldn't it be com.ubuntu.reminders-app.pot ?

Revision history for this message
Michael Zanetti (mzanetti) wrote :

The strings on the actions are a bit too long:
http://i.imgur.com/VQUqqFz.png

review: Needs Fixing
Revision history for this message
Sergio Schvezov (sergiusens) wrote :

On 24/01/14 08:27, Michael Zanetti wrote:
> Nice branch. Thanks.
>
> One comment:
>> - Makes the cmake rule to update the .pot file work (run it with `make com.ubuntu.reminders.pot`)
> shouldn't it be com.ubuntu.reminders-app.pot ?

We are trying to get rid of the -app from all the app names

Revision history for this message
Michael Zanetti (mzanetti) wrote :

>
> On 24/01/14 08:27, Michael Zanetti wrote:
> > Nice branch. Thanks.
> >
> > One comment:
> >> - Makes the cmake rule to update the .pot file work (run it with `make
> com.ubuntu.reminders.pot`)
> > shouldn't it be com.ubuntu.reminders-app.pot ?
>
> We are trying to get rid of the -app from all the app names

Ah ok. Still, I guess it would make sense to have it aligned to the rest of the app for now and it should be renamed when everything in the app is renamed to just "reminders".

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

- The com.ubuntu.reminders.pot name comes from $PROJECT_NAME in cmake, so I think we should be ok. Is there any place we need to still remove the -app suffix?
- I've worked around the fact that longer strings overlap in toolbar actions by shortening them to the minimum. I still think this is a bug that should be fixed in the SDK, though.

Revision history for this message
Sergio Schvezov (sergiusens) wrote :

On 24/01/14 09:09, Michael Zanetti wrote:
>> On 24/01/14 08:27, Michael Zanetti wrote:
>>> Nice branch. Thanks.
>>>
>>> One comment:
>>>> - Makes the cmake rule to update the .pot file work (run it with `make
>> com.ubuntu.reminders.pot`)
>>> shouldn't it be com.ubuntu.reminders-app.pot ?
>> We are trying to get rid of the -app from all the app names
> Ah ok. Still, I guess it would make sense to have it aligned to the rest of the app for now and it should be renamed when everything in the app is renamed to just "reminders".

Everything has been renamed, everything I knew of; manifest, desktop,
packagename. All controlled from the main CMakeLists.txt

Hmm, now that you bring it up, might be missing the applicationName in
the qml.

Revision history for this message
Michael Zanetti (mzanetti) wrote :

On Friday 24 January 2014 13:06:26 you wrote:
> On 24/01/14 09:09, Michael Zanetti wrote:
> >> On 24/01/14 08:27, Michael Zanetti wrote:
> >>> Nice branch. Thanks.
> >>>
> >>> One comment:
> >>>> - Makes the cmake rule to update the .pot file work (run it with `make
> >>
> >> com.ubuntu.reminders.pot`)
> >>
> >>> shouldn't it be com.ubuntu.reminders-app.pot ?
> >>
> >> We are trying to get rid of the -app from all the app names
> >
> > Ah ok. Still, I guess it would make sense to have it aligned to the rest
> > of the app for now and it should be renamed when everything in the app is
> > renamed to just "reminders".
> Everything has been renamed, everything I knew of; manifest, desktop,
> packagename. All controlled from the main CMakeLists.txt
>
> Hmm, now that you bring it up, might be missing the applicationName in
> the qml.

grepping through the code this seems to be missing

* src/app/reminders-app.desktop
* debian package
* run_on_ubuntu_touch script
* main qml file
* autopilot test suite name
* copyright headers
* applicationName

Revision history for this message
Michael Zanetti (mzanetti) wrote :

as agreed on IRC we'll get this merged as is (after David pushes the last fixes) and we'll have a cleanup round soon

Revision history for this message
Michael Zanetti (mzanetti) wrote :

oh... they're already there by now... lgtm now

review: Approve
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Francis Ginther (fginther) wrote :

Rejecting because jenkins can't automatically merge this due to:
bzrlib.errors.UnknownErrorFromSmartServer: Server sent an unexpected error: ('error', 'GhostRevisionsHaveNoRevno', 'Could not determine revno for {<email address hidden>} because its ancestry shows a ghost at {<email address hidden>}')

See also: http://wiki.bazaar.canonical.com/GhostRevision

Recommendation is to copy the changes to a new branch and resubmit.

I've rejected the MP to prevent jenkins from further attempts to merge this.

Unmerged revisions

33. By David Planella

Updated .pot file

32. By David Planella

Work around long strings overlapping in the actions, fix top cmake file to list i18n sources

31. By David Planella

Updated POT template file

30. By David Planella

Fixed a translations plural form

29. By David Planella

Translations fixes: marked a few strings for translations, fixed .pot file rule, capitalized some strings

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches