Please take a look.
https://codereview.appspot.com/6846132/diff/1/config.yaml File config.yaml (right): https://codereview.appspot.com/6846132/diff/1/config.yaml#newcode16 config.yaml:16: default: 8081 On 2012/11/30 14:32:18, gary.poster wrote: > Breaking news: Kapil has requested 8080 (as of yesterday). We will serve the > GUI from port 80 after bug 1083545, and 443 (by default) after bug 1083920. > Might as well go ahead and change to the desired 8080 here now, while we are > thinking of it. Done. https://codereview.appspot.com/6846132/diff/1/config/juju-api-improv.conf.template File config/juju-api-improv.conf.template (left): https://codereview.appspot.com/6846132/diff/1/config/juju-api-improv.conf.template#oldcode9 config/juju-api-improv.conf.template:9: exec /usr/bin/python %(juju_dir)s/improv.py -f %(juju_dir)s/sample.json &> /tmp/improv-agent.output On 2012/11/30 14:32:18, gary.poster wrote: > Why don't you stash the output any more? Doesn't seem critical also seems > reasonably nice. Ah! Sorry for not mentioning it in the MP description. The redirection seems to confuse upstart (it is not able to grab the PID). Moreover, output and errors are printed by default in /var/log/upstart/, which seems the place where people expect logs to be placed. So I decided to remove the redirection without hesitation. https://codereview.appspot.com/6846132/diff/1/hooks/start File hooks/start (right): https://codereview.appspot.com/6846132/diff/1/hooks/start#newcode39 hooks/start:39: log('Generating the Juju-GUI configuration file.') On 2012/11/30 14:32:18, gary.poster wrote: > Niggly niggle: you use "Juju GUI" in your new log message. I like that better > than "Juju-GUI" myself, but might as well make things consistent. If you feel > like it. :-) Done. s/Juju-GUI/Juju GUI/ everywhere. https://codereview.appspot.com/6846132/diff/1/tests/deploy.test File tests/deploy.test (right): https://codereview.appspot.com/6846132/diff/1/tests/deploy.test#newcode88 tests/deploy.test:88: self.addCleanup( On 2012/11/30 14:32:18, gary.poster wrote: > Please add this before the test itself (check_service in this case). This means > that if the test fails, stop_services still is called. It also is nice that the > actual test is the last line in the function IMO, but that's less important. Done. https://codereview.appspot.com/6846132/diff/1/tests/deploy.test#newcode98 tests/deploy.test:98: self.addCleanup( On 2012/11/30 14:32:18, gary.poster wrote: > Please add this before the test itself Done. https://codereview.appspot.com/6846132/diff/1/tests/deploy.test#newcode109 tests/deploy.test:109: self.addCleanup( On 2012/11/30 14:32:18, gary.poster wrote: > Please add this before the test itself Done. https://codereview.appspot.com/6846132/ -- https://code.launchpad.net/~frankban/charms/precise/juju-gui/bug-1074412-real-env/+merge/137140 Your team Juju GUI Hackers is requested to review the proposed merge of lp:~frankban/charms/precise/juju-gui/bug-1074412-real-env 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
