← Back to team overview

yellow team mailing list archive

Re: Updated the charm to serve Juju GUI releases. (issue 6977043)

 

Hi Nicola,

thanks for the review.


https://codereview.appspot.com/6977043/diff/8001/hooks/config-changed
File hooks/config-changed (right):

https://codereview.appspot.com/6977043/diff/8001/hooks/config-changed#newcode50
hooks/config-changed:50: # Restarting of the gui and api services is
handled below.
On 2012/12/20 13:09:14, teknico wrote:
> The two lines above could be replaced by:
> # Fetch new sources if needed, and restart GUI and API services

This pre-existed and I think it's actually correct, the services are not
restarted here.

https://codereview.appspot.com/6977043/diff/8001/hooks/utils.py
File hooks/utils.py (right):

https://codereview.appspot.com/6977043/diff/8001/hooks/utils.py#newcode310
hooks/utils.py:310: """Set up nginx."""
On 2012/12/20 13:09:14, teknico wrote:
> Well, nginx has nothing to do with the API environment, has it? It
looks like
> its setup code should be part of setup_gui, and setup_api should be
empty, for
> now. Shortly we will copy the nginx SSL certificate and private key to
the API
> environment, that's one thing that should go in setup_api.

Good catch, I'll rename this function.

https://codereview.appspot.com/6977043/diff/8001/tests/test_utils.py
File tests/test_utils.py (right):

https://codereview.appspot.com/6977043/diff/8001/tests/test_utils.py#newcode117
tests/test_utils.py:117: """Simulates a Launchpad hosted file returned
by launchpadlib."""
On 2012/12/20 13:09:14, teknico wrote:
> "Simulate..."

Done.

https://codereview.appspot.com/6977043/

-- 
https://code.launchpad.net/~frankban/charms/precise/juju-gui/bug-1088618-serve-releases/+merge/140745
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~frankban/charms/precise/juju-gui/bug-1088618-serve-releases into lp:~juju-gui/charms/precise/juju-gui/trunk.


References