Code review comment for lp:~diegosarmentero/ubuntuone-windows-installer/ui-improves

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

(01:08:39 PM) nessita: gatox: so, as an exercise, can you please change on_add_storage_button_clicked to use inlineCallbacks + yield instead of:
(01:08:39 PM) nessita: credtool = CredentialsManagementTool()
(01:08:39 PM) nessita: d = credtool.find_credentials()
(01:08:39 PM) nessita: d.addCallback(open_url)

Also, these imports

from twisted.internet import defer
from twisted.internet import reactor

from PyQt4 import QtGui
from PyQt4 import QtCore

should be:

from twisted.internet import defer, reactor
from PyQt4 import QtGui, QtCore

* This code is not needed:

    def tearDown(self):
        """Clean up the reactor."""
        BaseTestCase.tearDown(self)

* This assert has not effect!

        self.assertTrue(True)

* This code:

    def account_info(*args):
        d = defer.Deferred()
        reactor.callLater(.1, d.callback, {"quota_total": 1000})
        return d

should be:

    def account_info(*args):
        return defer.succeed({"quota_total": 1000})

* The branch lacks of tests for the new modules local_folders.py and preferences.py.

* Why you removed the LocalFoldersTestCase?

* Also, test for local_folders.py is needed: I understand there was no former tests for this module, so I request just a test for the new functionality added/modified (show_hide_offer and on_add_storage_button_clicked).

* Question, why did you remove this code? set_button_setup_account_property

review: Needs Fixing

« Back to merge proposal