[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