Code review comment for lp://qastaging/~bac/juju-quickstart/get_os_version

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

LGTM with minors.
Thank you!

https://codereview.appspot.com/99410054/diff/20001/quickstart/platform_support.py
File quickstart/platform_support.py (right):

https://codereview.appspot.com/99410054/diff/20001/quickstart/platform_support.py#newcode22
quickstart/platform_support.py:22: )
In the other quickstart modules we separate the __future__ imports from
the other ones, considering them a special case. This is not required by
PEP8, so change it only if you like the idea.

https://codereview.appspot.com/99410054/diff/20001/quickstart/tests/test_manage.py
File quickstart/tests/test_manage.py (right):

https://codereview.appspot.com/99410054/diff/20001/quickstart/tests/test_manage.py#newcode920
quickstart/tests/test_manage.py:920: 'or
environments.yaml'.format(settings.JUJU_HOME))
Thank you!

https://codereview.appspot.com/99410054/diff/20001/requirements.pip
File requirements.pip (right):

https://codereview.appspot.com/99410054/diff/20001/requirements.pip#newcode26
requirements.pip:26: # here when moving to a newer jujuclient that fixes
that does correctly
s/that fixes //

https://codereview.appspot.com/99410054/diff/20001/requirements.pip#newcode32
requirements.pip:32:
Blank line?

https://codereview.appspot.com/99410054/

« Back to merge proposal