Code review comment for lp://qastaging/~diegosarmentero/ubuntuone-control-panel/menu-desktop-services-actions

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

« Back to merge proposal