Hi Francesco, Approved with fixes. (I think that is one of the new things we're supposed to say. :)
I have a few comments, mainly documentation. The charm looks very good. Reading through it I remembered how nice the python-shelltoolbox is! Thanks. https://codereview.appspot.com/6846132/diff/1/README.txt File README.txt (right): https://codereview.appspot.com/6846132/diff/1/README.txt#newcode71 README.txt:71: Where does this command run the tests? ec2? Or whatever the default environment is? (Looks like the latter.) You should state that so there is no confusion. https://codereview.appspot.com/6846132/diff/1/config.yaml File config.yaml (right): https://codereview.appspot.com/6846132/diff/1/config.yaml#newcode23 config.yaml:23: description: | I'm confused as to what this option causes to happen. Do you mean it connects to the improv script running on uistage.jujucharms.com? I think so but the description could be more explicit. https://codereview.appspot.com/6846132/diff/1/config/juju-api-improv.conf.template File config/juju-api-improv.conf.template (right): https://codereview.appspot.com/6846132/diff/1/config/juju-api-improv.conf.template#newcode2 config/juju-api-improv.conf.template:2: author "Canonical" We are using 'improv' as if it has some inherent meaning, which I'm not sure is true. It is OK that Kapil called the script that but for us to propagate it in user-facing documentation seems wrong. 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. -- Mailing list: https://launchpad.net/~yellow Post to : [email protected] Unsubscribe : https://launchpad.net/~yellow More help : https://help.launchpad.net/ListHelp
