Code review comment for lp:~artmello/webbrowser-app/webbrowser-app-fix_1484562

Revision history for this message
Olivier Tilloy (osomon) wrote :

In the two new test functions, checking that the 'count' property of a spy is 0 right after calling clear() on that spy is useless. This is guaranteed to be true, by definition.

In onDeletePressed, if the left panel has active focus and the current index is 0 ("All History"), you could call historyModel.clearAll() instead of iterating through all the entries to delete them one by one.

Similarly, it might be interesting to add a 'clearByDate()' method to the history model to allow deleting all the entries for one given date, instead of iterating. This would be faster and cleaner.

review: Needs Fixing

« Back to merge proposal