[Merge] lp:~uriboni/webbrowser-app/keyboard-navigation into lp:webbrowser-app

Olivier Tilloy olivier.tilloy at canonical.com
Thu Jun 11 18:22:59 UTC 2015


Review: Needs Fixing

We’re getting there nicely. A few more minor remarks:

288	+                if (tabsModel.count >= 0) {

It should be (tabsModel.count > 0), there is no point in trying to close the current tab if there isn’t any.


680	+    def setUp(self, url="/test1"):

That argument would better be named 'path', rather than 'url'.


986	+        self.assertThat(suggestions.count, Eventually(NotEquals(0)))

Although functionally equivalent, GreaterThan would be more appropriate than NotEquals.


996	+    def test_keyboard_movement(self):

Can test_keyboard_movement be renamed test_keyboard_navigation ?


1085	+        QCOMPARE(item.count(), 0);

Can I suggest QVERIFY(item.isEmpty()) ?
-- 
https://code.launchpad.net/~uriboni/webbrowser-app/keyboard-navigation/+merge/260183
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app.



More information about the Ubuntu-reviews mailing list