← Back to team overview

yellow team mailing list archive

Re: Disable console (issue 6856112)

 

Looks good to me.
I have just one minor below, but I think this can be landed.


https://codereview.appspot.com/6856112/diff/5001/hooks/start
File hooks/start (right):

https://codereview.appspot.com/6856112/diff/5001/hooks/start#newcode23
hooks/start:23: import json
I usually separate imports in 3 groups: stdlib (see os and sys above),
other installed libraries (e.g. charmhelpers) and application ones
(utils). Inside each group, libraries are imported in alphabetical
order. json should be placed in the first group. Anyway, this is just a
suggestion, do with it what you want, I am not sure we have a code style
for that.

https://codereview.appspot.com/6856112/diff/5001/hooks/start#newcode51
hooks/start:51: 'console_enabled':
json.dumps(config['juju-gui-console-enabled'])
Really nice.

https://codereview.appspot.com/6856112/

-- 
https://code.launchpad.net/~makyo/charms/precise/juju-gui/juju-gui/+merge/136985
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~makyo/charms/precise/juju-gui/juju-gui into lp:~juju-gui/charms/precise/juju-gui/trunk.


References