I was just reviewing where we were and had two comments. No change to my previous review/blessing. :-)
Gary 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 I think the point of the comment is to reassure readers that these things will be restarted, just not in this code block. "below" means "not in this code block." The comment worked for me. Would this have been clearer for you? "# The juju_gui_source_changed and juju_api_branch_changed variables control whether we restart the GUI and the API, respectively, at the end of the function." Or something. Other ideas? 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 point. 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. -- Mailing list: https://launchpad.net/~yellow Post to : [email protected] Unsubscribe : https://launchpad.net/~yellow More help : https://help.launchpad.net/ListHelp
