[Merge] lp:~boiko/dialer-app/autopilot_upstart into lp:dialer-app
Leo Arias
leo.arias at canonical.com
Thu May 22 07:13:39 UTC 2014
Review: Approve
<elopio> boiko: :)
<elopio> what branch are you talking about? the upstart one, or you have a new one?
<elopio> ok! it's passing now!
<boiko> the upstart one
<elopio> the only thing I don't like is
<elopio> 58 + sys.stderr.write('Failed to online the modem phonesim.!\n')
<elopio> 59 + sys.exit(1)
<boiko> so, what do you suggest in that case?
<elopio> the tests shouldn't write to command line, because that can get lost on the runner and not get to the jenkins server that collects the results
<elopio> can you just raise an exception?
-*- boiko is not really fluent in python :)
<elopio> that will make the test fail, and it will collect the error message for sure.
<boiko> maybe not even handle the exception there?
<elopio> right, just let it go to the test.
<boiko> yep, ok
-*- boiko fixes it
<elopio> I mean, if you can provide a better error message than the one check call throws, it's ok to wrap it on one of your own. If not, it's ok to just let it propagate.
<elopio> boiko: also, with this + while phonesim['Online'] == 0:
<elopio> can you cause an infinte loop?
<boiko> good point, the only case that could happen is if there is a bug in ofono itself
<boiko> but I can set a maximum of 5 or 10 tries
<elopio> boiko: that would be safer, yes.
<boiko> I'll fix that too
<elopio> great, thanks.
<elopio> I'll leave my approval on the branch
--
https://code.launchpad.net/~boiko/dialer-app/autopilot_upstart/+merge/214963
Your team Ubuntu Phablet Team is requested to review the proposed merge of lp:~boiko/dialer-app/autopilot_upstart into lp:dialer-app.
More information about the Ubuntu-reviews
mailing list