Merge lp://qastaging/~nataliabidart/ubuntuone-control-panel/decouple-devices into lp://qastaging/ubuntuone-control-panel

Proposed by Natalia Bidart
Status: Merged
Approved by: Natalia Bidart
Approved revision: 120
Merged at revision: 117
Proposed branch: lp://qastaging/~nataliabidart/ubuntuone-control-panel/decouple-devices
Merge into: lp://qastaging/ubuntuone-control-panel
Diff against target: 254 lines (+127/-54)
3 files modified
ubuntuone/controlpanel/backend.py (+75/-40)
ubuntuone/controlpanel/tests/__init__.py (+14/-12)
ubuntuone/controlpanel/tests/test_backend.py (+38/-2)
To merge this branch: bzr merge lp://qastaging/~nataliabidart/ubuntuone-control-panel/decouple-devices
Reviewer Review Type Date Requested Status
Manuel de la Peña (community) Approve
Alejandro J. Cura (community) Approve
Review via email: mp+55345@code.qastaging.launchpad.net

Commit message

- Decoupled device list retrieved from the web from the local settings retrieved from syncdaemon (LP: #720704).

Description of the change

This change decouples the retrieval of the device info list from the retrieval of local settings from syncdaemon.

To test this, you should:

killall ubuntuone-control-panel-backend

In two terminals, run:

DEBUG=True PYTHONPATH=. bin/ubuntuone-control-panel-backend
DEBUG=True PYTHONPATH=. bin/ubuntuone-control-panel-gtk

The first test would be disconnecting your network connection and going to the devices tab, you should see a single device (your local device).

The second test is more complex, since it requires that syncdaemon is disabled. Due to bug bug #744980, you need to:

* u1sdtool -q
* edit ~/.config/ubuntuone/syncdaemon.conf and set files_sync_enabled = False
* re-run the control panel backend and frontend
* go to the devices tab and confirm that the device list is retrieved, but no local settings are offered

To post a comment you must log in.
119. By Natalia Bidart

Fixed typo.

Revision history for this message
Alejandro J. Cura (alecu) wrote :

Nice branch!

review: Approve
Revision history for this message
Manuel de la Peña (mandel) wrote :

Work fine with me to small remarks:

1. 'more the devices will be configurable.' - I guess is either 'more devices' or 'more of the devices'

2. I'm confused about this:
118 + show_notifs = bool_str(show_notifs)
119 + local_device["show_all_notifications"] = show_notifs

Why reassigning show_notifs and then storing it in the local_devices dict. What about just assigning it to the dict directly?

Since I don't consider the two above things to be lowering the quality of the code I approve and will trust the committer to the right thing.

review: Approve
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

> Work fine with me to small remarks:
>
> 1. 'more the devices will be configurable.' - I guess is either 'more devices'
> or 'more of the devices'

Fixed, thanks for the attention to detail!

> 2. I'm confused about this:
> 118 + show_notifs = bool_str(show_notifs)
> 119 + local_device["show_all_notifications"] = show_notifs
>
> Why reassigning show_notifs and then storing it in the local_devices dict.
> What about just assigning it to the dict directly?

Because otherwise it fells over the 80-columns width. I personally prefer assigning to a local variable that chopping off the line with = \.

120. By Natalia Bidart

Another typo, thanks mandel!

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