Code review comment for lp:~frankban/juju-quickstart/jujucharms-bundles

Revision history for this message
Francesco Banconi (frankban) wrote :

Please take a look.

https://codereview.appspot.com/207040043/diff/1/HACKING.rst
File HACKING.rst (right):

https://codereview.appspot.com/207040043/diff/1/HACKING.rst#newcode228
HACKING.rst:228: juju quickstart -e local mediawiki-single
On 2015/02/27 03:00:09, rharding wrote:
> <3 nice pretty command

Indeed!

https://codereview.appspot.com/207040043/diff/1/quickstart/__init__.py
File quickstart/__init__.py (right):

https://codereview.appspot.com/207040043/diff/1/quickstart/__init__.py#newcode48
quickstart/__init__.py:48: VERSION = (1, 7, 0)
On 2015/02/27 03:00:10, rharding wrote:
> what do you think of a 2.0? I guess it's not backward incompatible but
wondering
> if new charmstore/etc will be worthy of a big version update?

Sounds good, bumped version up to 2.0.

https://codereview.appspot.com/207040043/diff/1/quickstart/juju.py
File quickstart/juju.py (right):

https://codereview.appspot.com/207040043/diff/1/quickstart/juju.py#newcode68
quickstart/juju.py:68: params['Version'] = version
On 2015/02/27 03:00:10, rharding wrote:
> so this seems like a change to the call to the charm? Is Version going
to be
> supported in the charm then and name no longer required as a param?
This is the
> guiserver bits correct?

Yes this is a call to the GUI server.
Version is already supported in the charm, as implemented by Madison.
Name has always been optional, only required when sending a YAML with
more than one bundle, which is never the case in quickstart.

https://codereview.appspot.com/207040043/

« Back to merge proposal