Ubuntu Clock App

Merge lp://qastaging/~gang65/ubuntu-clock-app/ubuntu-clock-app-australia-display-fix into lp://qastaging/ubuntu-clock-app

  1. ubuntu-clock-app-australia-display-fix
  2. Merge into trunk
Proposed by Bartosz Kosiorek
Status: Work in progress
Proposed branch: lp://qastaging/~gang65/ubuntu-clock-app/ubuntu-clock-app-australia-display-fix
Merge into: lp://qastaging/ubuntu-clock-app
Diff against target: 78 lines (+29/-22)
2 files modified
app/components/DigitalMode.qml (+28/-22)
debian/changelog (+1/-0)
To merge this branch: bzr merge lp://qastaging/~gang65/ubuntu-clock-app/ubuntu-clock-app-australia-display-fix
Reviewer Review Type Date Requested Status
Bartosz Kosiorek Needs Information
Nekhelesh Ramananthan Disapprove
Victor Thompson Needs Information
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Review via email: mp+274183@code.qastaging.launchpad.net

Commit message

Fix digital time display for English Australia locale (LP: #1384739)

Description of the change

Fix digital time display for English Australia locale (LP: #1384739)

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

PASSED: Continuous integration, rev:398
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/851/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/196

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

review: Approve (continuous-integration)
Reply
Revision history for this message
Victor Thompson (vthompson) wrote :

2 inline comments.

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

As per the inline comments, I do think that this is a upstream QT issue IMO. I also commented about it in the bug report at https://bugs.launchpad.net/ubuntu-clock-app/+bug/1384739/comments/5. By patching it downstream in the clock app, I fear we might break other locales without proper testing.

Personally I don't like introducing downstream patches when the correct solution breaks in certain use-cases due to upstream bugs.

review: Disapprove
Reply
Revision history for this message
Bartosz Kosiorek (gang65) wrote :

Ok. I understand that it needs extensive testing.

Can we move this patch to the next Clock release then?

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

> Ok. I understand that it needs extensive testing.
>
> Can we move this patch to the next Clock release then?

It would be better if the fix for this issue came from QtUbuntu [1] rather than patching it in the clock app. Downstream QT Patches are done in QtUbuntu (and upstream after a while). Can you talk to Timo Jyrinki who usually does this? You can find him in #ubuntu-app-devel, #ubuntu-touch etc under the nickname Mirv.

Let's not add these kind of patches in clock app due to upstream issues *unless* it is a High/Critical issue that affects a large portion of users. Instead we should convince upstream to fix these issues.

Testing is just one aspect of it. It just adds the headache of maintaining that patch ourselves once we add it.

[1] https://launchpad.net/qtubuntu

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

I have updated the bug report to reflect the decision and the way to move forward with this issue.

Reply

Unmerged revisions

398. By Bartosz Kosiorek

Fix digital time display for English Australia locale (LP: #1384739)

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