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

Revision history for this message
Richard Harding (rharding) wrote :

Thanks for this branch. It looks really solid and loving the new
features.

I'm not a huge fan of the 'reference' but can't think of a great
replacement word so oh well.

I've got one question on the rpc call changes to the gui server.

Heads up that Kapil updated the python-jujuclient tonight and says
"pushed new py j client w/ iterator fix .. version incr math fixed as
well (0.50.1)"

So if you get time in the morning please see if you can do a round of QA
with that and we need to look at what it'll take to get that into the
stable PPA. I mentioned to him how we'd be able to help packaging and I
think the tox work done here in quickstart might be useful for
python-jujuclient and the deployer in the future.

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
<3 nice pretty command

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)
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?

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
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?

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

https://codereview.appspot.com/207040043/diff/1/quickstart/manage.py#newcode399
quickstart/manage.py:399: ''.format(jujucharms=settings.JUJUCHARMS_URL))
<3 this looks great.

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

https://codereview.appspot.com/207040043/diff/1/quickstart/tests/test_juju.py#newcode207
quickstart/tests/test_juju.py:207: 'Version': 4,
Yea, so this is where I'm curious on the changes on the gui charm end.

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

« Back to merge proposal