yellow team mailing list archive

[Francesco Banconi <francesco.banconi@xxxxxxxxxxxxx>]

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.