← Back to team overview

ubuntu-touch-coreapps-reviewers team mailing list archive

Re: [Merge] lp:~gang65/ubuntu-clock-app/ubuntu-clock-app-alarm-order-fix into lp:ubuntu-clock-app

 

Review: Approve

> This change fixes multiple changes:
> 1. Passed alarms (when you disable phone and alarm passed), is enabled by
> default, and it is not possible to disable it (because the date is wrong)

I understand now better. So the alarm.save() operation is failing because the alarm date is incorrect after changing the timezone. So you want to fix the alarm date and then disable the alarm. Sounds fair. Approving on this reasoning.


> 2. According to Clock documentation, the alarms should be sorted by time of
> occurence (even if it was disabled). It is done automatically by SDK via alarm
> time. If we do not update disabled alarm then order of such alarm is wrong.
> 

This reasoning is however incorrect. Alarms *are not* sorted by time of occurence.In fact the SDK sorts it using a combination of date and alarm ID which is why https://bugs.launchpad.net/ubuntu/+source/ubuntu-ui-toolkit/+bug/1439393 was reported.

> If you have some spare time I could explain it to you on Hangout.

Not required. I understand the patch better now. Nice catch and thanks for the quick fix.
-- 
https://code.launchpad.net/~gang65/ubuntu-clock-app/ubuntu-clock-app-alarm-order-fix/+merge/274270
Your team Ubuntu Clock Developers is subscribed to branch lp:ubuntu-clock-app.


References