yellow team mailing list archive

[Nicola Larosa <nicola.larosa@xxxxxxxxxxxxx>]

Land with changes.

Very nice refactoring job and new code, thanks.

A few small changes below, plus one code placement objection. Also, I
personally prefer that ".tar.gz" extension over the ".tgz" one, but I
guess it's personal taste.


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.
The two lines above could be replaced by:
# Fetch new sources if needed, and restart GUI and API services

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."""
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.

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."""
"Simulate..."

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.