Ubuntu One Control Panel

Merge lp://qastaging/~diegosarmentero/ubuntuone-control-panel/menu-desktop-services-actions into lp://qastaging/ubuntuone-control-panel

  1. menu-desktop-services-actions
  2. Merge into trunk
Proposed by Diego Sarmentero
Status: Merged
Approved by: Diego Sarmentero
Approved revision: 349
Merged at revision: 343
Proposed branch: lp://qastaging/~diegosarmentero/ubuntuone-control-panel/menu-desktop-services-actions
Merge into: lp://qastaging/ubuntuone-control-panel
Prerequisite: lp://qastaging/~diegosarmentero/ubuntuone-control-panel/refactor-sync-status
Diff against target: 539 lines (+397/-54) (has conflicts)
2 files modified
ubuntuone/controlpanel/gui/qt/systray.py (+194/-51)
ubuntuone/controlpanel/gui/qt/tests/test_systray.py (+203/-3)
Text conflict in ubuntuone/controlpanel/gui/qt/systray.py
Text conflict in ubuntuone/controlpanel/gui/qt/tests/test_systray.py
To merge this branch: bzr merge lp://qastaging/~diegosarmentero/ubuntuone-control-panel/menu-desktop-services-actions
Reviewer Review Type Date Requested Status
Brian Curtin (community) Approve
Manuel de la Peña (community) Approve
Review via email: mp+119370@code.qastaging.launchpad.net

Commit message

- Adding open (folder, program, url) actions to the menu (LP: #1034542)

To post a comment you must log in.
Revision history for this message
Manuel de la Peña (mandel) wrote :

Some small fixes:

icon is always not None, I suppose it is icon_name

127 + icon = QtGui.QIcon()
128 + if icon is not None:
129 + icon = icon_from_name(icon_name)
130 + text = data.get('msg')
131 + self.status.setIcon(icon)
132 + self.status.setText(text)

Possible attr error:

135 + self._backend_method = getattr(self.backend, data['backend_method'])

A little cleaner way:

326 + @inlineCallbacks
327 + def test_refresh_status(self):
328 + """Test that it receives the proper information from syncdaemon."""
329 + yield self.ui.refresh_status()

do instead:

def test_refresh_status(self):
    """Test that it receives the proper information from syncdaemon."""
    return self.ui.refresh_status()

Tests can return deferreds so no need for the inlineCallbacks

The following is a personal thing, ignore it if you wish:

signal_pairs = (
    (self.status_action, self.change_status)
    (self.open_u1, self.restore_window)
    (self.open_u1_folder, self.open_u1_folder_action)
    (self.get_more_storage, self.get_more_storage_action)
    (self.go_to_web, self.go_to_web_action)
    (self.get_help_online, self.get_help_action)
    (self.quit, self.stop)
)

for button, callback in signal_pairs:
    button.triggered.connect(callback)

Instead of:

# Connect Signals
self.status_action.triggered.connect(self.change_status)
self.open_u1.triggered.connect(self.restore_window)
self.open_u1_folder.triggered.connect(self.open_u1_folder_action)
self.get_more_storage.triggered.connect(self.get_more_storage_action)
self.go_to_web.triggered.connect(self.go_to_web_action)
self.get_help_online.triggered.connect(self.get_help_action)
self.quit.triggered.connect(self.stop)

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

Looks good to me.

review: Approve
Revision history for this message
Brian Curtin (brian.curtin) wrote :

Agree on the last part with iterating over signal_pairs.

Revision history for this message
Brian Curtin (brian.curtin) wrote :

You still have TODO's on lines 73, 79, and 81.

review: Needs Fixing
Revision history for this message
Diego Sarmentero (diegosarmentero) wrote :

> You still have TODO's on lines 73, 79, and 81.

Yes, those are for the future branches i'm working right now

Revision history for this message
Brian Curtin (brian.curtin) :
review: Approve
Revision history for this message
Ubuntu One Auto Pilot (otto-pilot) wrote :
Download full text (147.2 KiB)

The attempt to merge lp:~diegosarmentero/ubuntuone-control-panel/menu-desktop-services-actions into lp:ubuntuone-control-panel failed. Below is the output from the failed tests.

*** Running DBus test suite ***
ubuntuone.controlpanel.dbustests.test_dbus_service
  BaseTestCase
    runTest ... [OK]
  DBusServiceMainTestCase
    test_dbus_service_cant_register ... Control panel backend already running.
                                   [OK]
    test_dbus_service_main ... [OK]
  DBusServiceTestCase
    test_cant_register_twice ... [SKIPPED]
    test_dbus_busname_created ... [OK]
    test_error_handler_default ... [OK]
    test_error_handler_with_exception ... [OK]
    test_error_handler_with_failure ... [OK]
    test_error_handler_with_non_string_dict ... [OK]
    test_error_handler_with_string_dict ... [OK]
    test_register_service ... [OK]
  FileSyncTestCase
    test_file_sync_status_changed ... [OK]
    test_file_sync_status_disabled ... [OK]
    test_file_sync_status_disconnected ... [OK]
    test_file_sync_status_error ... [OK]
    test_file_sync_status_idle ... [OK]
    test_file_sync_status_starting ... [OK]
    test_file_sync_status_stopped ... [OK]
    test_file_sync_status_syncing ... [OK]
    test_file_sync_status_unknown ... [OK]
    test_status_changed_handler ... [OK]
    test_status_changed_handler_after_status_requested ... [OK]
    test_status_changed_handler_after_status_requested_twice ... [OK]
  OperationsAuthErrorTestCase
    test_account_info_returned ... [OK]
    test_change_device_settings ... [OK]
    test_change_replication_settings ... [OK]
    test_change_volume_settings ... [OK]
    test_connect_files ... [OK]
    test_devices_info_returned ... [OK]
    test_disable_files ... [OK]
    test_disconnect_files ... [OK]
    test_enable_files ... [OK]
    test_remove_device ... [OK]
    test_replications_info ... [OK]
    test_restart_files ... ...

Revision history for this message
Ubuntu One Auto Pilot (otto-pilot) wrote :
Download full text (146.5 KiB)

The attempt to merge lp:~diegosarmentero/ubuntuone-control-panel/menu-desktop-services-actions into lp:ubuntuone-control-panel failed. Below is the output from the failed tests.

*** Running DBus test suite ***
ubuntuone.controlpanel.dbustests.test_dbus_service
  BaseTestCase
    runTest ... [OK]
  DBusServiceMainTestCase
    test_dbus_service_cant_register ... Control panel backend already running.
                                   [OK]
    test_dbus_service_main ... [OK]
  DBusServiceTestCase
    test_cant_register_twice ... [SKIPPED]
    test_dbus_busname_created ... [OK]
    test_error_handler_default ... [OK]
    test_error_handler_with_exception ... [OK]
    test_error_handler_with_failure ... [OK]
    test_error_handler_with_non_string_dict ... [OK]
    test_error_handler_with_string_dict ... [OK]
    test_register_service ... [OK]
  FileSyncTestCase
    test_file_sync_status_changed ... [OK]
    test_file_sync_status_disabled ... [OK]
    test_file_sync_status_disconnected ... [OK]
    test_file_sync_status_error ... [OK]
    test_file_sync_status_idle ... [OK]
    test_file_sync_status_starting ... [OK]
    test_file_sync_status_stopped ... [OK]
    test_file_sync_status_syncing ... [OK]
    test_file_sync_status_unknown ... [OK]
    test_status_changed_handler ... [OK]
    test_status_changed_handler_after_status_requested ... [OK]
    test_status_changed_handler_after_status_requested_twice ... [OK]
  OperationsAuthErrorTestCase
    test_account_info_returned ... [OK]
    test_change_device_settings ... [OK]
    test_change_replication_settings ... [OK]
    test_change_volume_settings ... [OK]
    test_connect_files ... [OK]
    test_devices_info_returned ... [OK]
    test_disable_files ... [OK]
    test_disconnect_files ... [OK]
    test_enable_files ... [OK]
    test_remove_device ... [OK]
    test_replications_info ... [OK]
    test_restart_files ... ...

lp://qastaging/~diegosarmentero/ubuntuone-control-panel/menu-desktop-services-actions updated
349. By Diego Sarmentero

fixing pep8 issue

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