Ubuntu Clock App

Merge lp://qastaging/~gang65/ubuntu-clock-app/ubuntu-clock-city-name-fix into lp://qastaging/ubuntu-clock-app

  1. ubuntu-clock-city-name-fix
  2. Merge into trunk
Proposed by Bartosz Kosiorek
Status: Merged
Approved by: Bartosz Kosiorek
Approved revision: 330
Merged at revision: 336
Proposed branch: lp://qastaging/~gang65/ubuntu-clock-app/ubuntu-clock-city-name-fix
Merge into: lp://qastaging/ubuntu-clock-app
Diff against target: 3872 lines (+982/-928)
51 files modified
app/alarm/AlarmModelComponent.qml (+1/-1)
app/alarm/AlarmPage.qml (+1/-1)
app/alarm/AlarmSettingsPage.qml (+1/-1)
app/alarm/AlarmUtils.qml (+1/-1)
app/components/AnalogMode.qml (+1/-1)
app/components/AnalogShadow.qml (+1/-1)
app/components/Background.qml (+1/-1)
app/components/Clock.qml (+1/-1)
app/components/ClockCircle.qml (+1/-1)
app/components/DigitalMode.qml (+1/-1)
app/components/DigitalShadow.qml (+1/-1)
app/components/EmptyState.qml (+1/-1)
app/components/Shadow.qml (+1/-1)
app/components/SubtitledListItem.qml (+1/-1)
app/upstreamcomponents/FakeHeader.qml (+1/-1)
app/upstreamcomponents/FastScroll.js (+1/-1)
app/upstreamcomponents/FastScroll.qml (+1/-1)
app/worldclock/AddWorldCityButton.qml (+1/-1)
app/worldclock/UserWorldCityDelegate.qml (+3/-3)
app/worldclock/UserWorldCityList.qml (+1/-1)
app/worldclock/WorldCityList.qml (+22/-18)
backend/modules/Alarm/Settings/alarmsettings.cpp (+1/-1)
backend/modules/Alarm/Settings/alarmsettings.h (+1/-1)
backend/modules/Alarm/Settings/backend.cpp (+1/-1)
backend/modules/Alarm/Settings/backend.h (+1/-1)
backend/modules/DateTime/backend.cpp (+1/-1)
backend/modules/DateTime/backend.h (+1/-1)
backend/modules/DateTime/datetime.cpp (+1/-1)
backend/modules/DateTime/datetime.h (+1/-1)
backend/modules/GeoLocation/backend.cpp (+1/-1)
backend/modules/GeoLocation/backend.h (+1/-1)
backend/modules/GeoLocation/geolocation.cpp (+1/-1)
backend/modules/GeoLocation/geolocation.h (+1/-1)
backend/modules/Timezone/backend.cpp (+1/-1)
backend/modules/Timezone/backend.h (+1/-1)
backend/modules/Timezone/generictimezonemodel.cpp (+28/-12)
backend/modules/Timezone/generictimezonemodel.h (+1/-1)
backend/modules/Timezone/jsontimezonemodel.cpp (+11/-10)
backend/modules/Timezone/jsontimezonemodel.h (+1/-1)
backend/modules/Timezone/statictimezonemodel.cpp (+327/-308)
backend/modules/Timezone/statictimezonemodel.h (+4/-1)
backend/modules/Timezone/timezonemodel.cpp (+18/-12)
backend/modules/Timezone/timezonemodel.h (+6/-4)
debian/changelog (+6/-3)
po/com.ubuntu.clock.pot (+516/-516)
tests/autopilot/ubuntu_clock_app/CMakePluginParser.py (+1/-1)
tests/autopilot/ubuntu_clock_app/fixture_setup.py (+1/-1)
tests/autopilot/ubuntu_clock_app/tests/__init__.py (+1/-1)
tests/autopilot/ubuntu_clock_app/tests/test_clock.py (+1/-1)
tests/unit/ClockTestCase.qml (+1/-1)
tests/unit/MockClockApp.qml (+1/-1)
To merge this branch: bzr merge lp://qastaging/~gang65/ubuntu-clock-app/ubuntu-clock-city-name-fix
Reviewer Review Type Date Requested Status
Nekhelesh Ramananthan Approve
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Niklas Wenzel (community) Needs Information
Review via email: mp+266153@code.qastaging.launchpad.net

Commit message

Translate city and country names after language switch (LP: #1477492)
Fix issue when unable to add city/country with apostrophes (LP: #1473074)

Description of the change

Translate city and country names after language switch (LP: #1477492)
Fix issue when unable to add city/country with apostrophes (LP: #1473074)
Save CityId in User database instead of localized name

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

FAILED: Continuous integration, rev:314
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~gang65/ubuntu-clock-app/ubuntu-clock-city-name-fix/+merge/266153/+edit-commit-message

http://91.189.93.70:8080/job/ubuntu-clock-app-ci/727/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/72

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/727/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

FAILED: Continuous integration, rev:316
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~gang65/ubuntu-clock-app/ubuntu-clock-city-name-fix/+merge/266153/+edit-commit-message

http://91.189.93.70:8080/job/ubuntu-clock-app-ci/728/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/73

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/728/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

PASSED: Continuous integration, rev:318
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/729/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/74

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/729/rebuild

review: Approve (continuous-integration)
Revision history for this message
Niklas Wenzel (nikwen) wrote :

Looks good to me. Well done! ;)

See my inline comments for a few remarks. Some of them are questions though as you might have a reason why you did things differently.
Nevertheless, I'd like someone who is more familiar with the clock app code to review this as well. However, from looking at the code, everything in there looks reasonable to me.

Thanks!

review: Needs Information
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

PASSED: Continuous integration, rev:320
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/734/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/79

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/734/rebuild

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

PASSED: Continuous integration, rev:321
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/737/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/82

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/737/rebuild

review: Approve (continuous-integration)
Revision history for this message
Nekhelesh Ramananthan (nik90) wrote :

I haven't yet reviewed the entire MP yet, but as a start could you update the copyright year of the file you have modified in this MP to 2015 as well. So something like "Copyright (C) 2014-2015 Canonical Ltd"

review: Needs Fixing
Revision history for this message
Nekhelesh Ramananthan (nik90) wrote :

On testing I observed the following issues,

First, language switching nicely switched the world city translations as well. Good Job on that! However there seems to be some regressions that have been introduced.

1. FastScroll is broken even when the phone language is English. I believe bug 1483880 was reported for fastscroll not working for non-latin characters. But my issue is reproducible even for latin characters which is a regression.

2. Search for a city using the search toolbar function is broken in all languages (Quite serious). In fact it doesn't even go online to search Ubuntu Geonames for a city not found in the list we ship with clock app.

3. In other languages, the static city list is not ordered alphabetically. I see the order like A, A`, P, C, D etc.

review: Needs Fixing (testing)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

PASSED: Continuous integration, rev:322
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/744/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/89

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/744/rebuild

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

PASSED: Continuous integration, rev:323
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/746/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/91

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/746/rebuild

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

PASSED: Continuous integration, rev:325
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/747/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/92

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/747/rebuild

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

PASSED: Continuous integration, rev:326
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/753/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/98

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/753/rebuild

review: Approve (continuous-integration)
Revision history for this message
Nekhelesh Ramananthan (nik90) wrote :

Just tested it now again and it works nicely without regressions and fixes the issues it set out to fix. Will approve after I finish my code review. Thanks for working on this!

Revision history for this message
Nekhelesh Ramananthan (nik90) :
review: Abstain
Revision history for this message
Nekhelesh Ramananthan (nik90) wrote :

Code looks good except for some really really minor quibbles that I have pointed out. Most of them cant be ignored if you wish to do so, however there were some spelling mistakes and confusion in the function names. Please fix them. Thanks.

review: Needs Fixing
Revision history for this message
Nekhelesh Ramananthan (nik90) wrote :

Added inline comments

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

PASSED: Continuous integration, rev:328
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/758/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/103

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/758/rebuild

review: Approve (continuous-integration)
Revision history for this message
Nekhelesh Ramananthan (nik90) wrote :

* Does the MP add/remove user visible strings? If Yes, has the pot file been
    updated?

* Does the MP change the UI? If Yes, has it been approved by design?

No UI Changes.

* Did you perform an exploratory manual test run of your code change and any
    related functionality?

Yes, tested on N4, #95. All world city features work as expected!

* If the MP fixes a bug or implements a feature, are there accompanying unit
    and autopilot tests?

No accompanying unit test, but that is acceptable for now.

* Is the clock app trunk buildable and runnable using Qtcreator?

Yes

* Was the debian changelog updated?

Yes, but is causing a code conflict. Please fix the debian changelog and then top-approve when ready!

* Was the copyright years updated if necessary?

Yes, more than sufficiently ;)

review: Approve
lp://qastaging/~gang65/ubuntu-clock-app/ubuntu-clock-city-name-fix updated
330. By Bartosz Kosiorek

Updated translation template

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