[Merge] lp:~brendan-donegan/phablet-tools/citrain-diff into lp:phablet-tools
♫ Robert Bruce Park ♫
robert.park at canonical.com
Fri Nov 14 20:34:26 UTC 2014
Review: Approve
Hey, this code is pretty good. I see short functions with few args each and i like it.
My only concern is that i saw os.chdir() used and I've just gone through a long nightmare of refactoring code that used it poorly. It would probably be better if you passed in the cwd argument to Popen rather than chdir around. I know it's somewhat trivial in this case since you only call it once, so feel free to ignore me... Just my personal preference.
--
https://code.launchpad.net/~brendan-donegan/phablet-tools/citrain-diff/+merge/241675
Your team Ubuntu Phablet Team is subscribed to branch lp:phablet-tools.
More information about the Ubuntu-reviews
mailing list