Looks good, thank you! Maybe make the os.path change I mention, and look at my logs and make sure they are healthy.
Otherwise approved. Gary https://codereview.appspot.com/6850052/diff/1/README.txt File README.txt (right): https://codereview.appspot.com/6850052/diff/1/README.txt#newcode21 README.txt:21: On 2012/11/19 18:57:34, bac wrote: > This doesn't actually work as unit.test has '.discover('.') but is being run > from above. Either unit.test should read '.discover('./tests') or it should be > run from within the tests directory. Maybe better would be to .discover(os.path.dirname(__file__)) https://codereview.appspot.com/6850052/diff/1/README.txt#newcode42 README.txt:42: autoreconf Before this I needed to sudo apt-get install libtool autoconf (and maybe some other bits too) https://codereview.appspot.com/6850052/diff/1/README.txt#newcode46 README.txt:46: On 2012/11/19 18:57:34, bac wrote: > Has anyone talked to Jim about packaging this into a PPA? It is supposed to land...sometime... https://codereview.appspot.com/6850052/diff/1/README.txt#newcode69 README.txt:69: On 2012/11/19 18:57:34, bac wrote: > Ugh, I seem to have lxc issues since upgrading to Quantal, so I cannot get it to > start now. If you want to pay, ec2 works, I think. The unit tests seem to run in a separate machine, which is seems unnecessarily annoying. Not a huge deal, and maybe I'm misinterpreting: http://pastebin.ubuntu.com/1371194/ Wow, running the tests is slow. :-/ I'd forgotten about this. My lxc results were as follows. They seem to show some unexpected problems. Are they OK? http://pastebin.ubuntu.com/1371234/ https://codereview.appspot.com/6850052/diff/1/README.txt#newcode71 README.txt:71: "setup-environment" is an easy way to get started. Maybe it would be worth mentioning that you might want to set your default environment to lxc? Maybe it would be worth mentioning that using an lxc environment requires that you install the apt-cacher-ng and lxc packages? Your call. https://codereview.appspot.com/6850052/diff/1/tests/unit.test File tests/unit.test (right): https://codereview.appspot.com/6850052/diff/1/tests/unit.test#newcode6 tests/unit.test:6: sys.exit(unittest.TextTestRunner().run(unittest.TestLoader().discover('.'))) On 2012/11/19 18:57:34, bac wrote: > See comment in README Mine too :-) https://codereview.appspot.com/6850052/ -- https://code.launchpad.net/~benji/charms/precise/juju-gui/second/+merge/134370 Your team Juju GUI Hackers is requested to review the proposed merge of lp:~benji/charms/precise/juju-gui/second into lp:~juju-gui/charms/precise/juju-gui/trunk. -- Mailing list: https://launchpad.net/~yellow Post to : [email protected] Unsubscribe : https://launchpad.net/~yellow More help : https://help.launchpad.net/ListHelp
