[Merge] lp:~om26er/messaging-app/use_fixture into lp:messaging-app

Leo Arias leo.arias at canonical.com
Fri Jul 11 00:52:35 UTC 2014


Review: Needs Fixing code review

Awesome! Thank you Omer. 

21	+ self.get_multiple_selection_list_view().count.wait_for(1)

In here, please call the count before the message is sent, like

original_number_of_messages = self.get_multiple_selection_list_view().count
[...]
self.get_multiple_selection_list_view().count.wait_for(original_number_of_messages + 1)

That way it works even if you want to send more than one message.

43	+import fixtures
44	+import subprocess
45	+import os

These needs to be sorted alphabetically, and following the pep8 rule that first we have the list of internal python modules:

import os
import subprocess

import fixtures

The rest is just so cool. You removed the duplication and put it in a reusable component. Nice work.
-- 
https://code.launchpad.net/~om26er/messaging-app/use_fixture/+merge/225350
Your team Ubuntu Phablet Team is requested to review the proposed merge of lp:~om26er/messaging-app/use_fixture into lp:messaging-app.



More information about the Ubuntu-reviews mailing list