Code review comment for lp:~frankban/juju-quickstart/unicode-all-the-things

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

Reviewers: mp+198112_code.launchpad.net,

Message:
Please take a look.

Description:
Use unicode everywhere in the application.

My apologies for this branch, the diff is long
and the contents themself are boring: I am very
sorry. Anyway, I think this pushes quickstart
in a better place re: unicode/bytes handling.

Everything here is almost mechanical, including
the annoying problem of having to store encoded
strings in the exceptions: please let me know
if you have any suggestions, or if I am missing
something.

The serializers module is new stuff: it allows
for loading YAML strings as unicode and for
dumping dictionaries in the extended format
(used e.g. by juju-core in the envs.yaml file).

To QA this, just use quickstart as usual, deploying
a bundle or spinning up an environment.
Then try to break it passing exotic strings as
options, e.g.:
.venv/bin/python juju-quickstart --gui-charm-url ☢
(FWIW the above character is \xe2\x98\xa2).
This obviously fail, but the error should not
be unicode related.

Thank you and sorry again.

https://code.launchpad.net/~frankban/juju-quickstart/unicode-all-the-things/+merge/198112

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/38500043/

Affected files (+372, -148 lines):
   A [revision details]
   M juju-quickstart
   M quickstart/__init__.py
   M quickstart/app.py
   M quickstart/juju.py
   M quickstart/manage.py
   M quickstart/models/charms.py
   M quickstart/models/envs.py
   A quickstart/serializers.py
   M quickstart/settings.py
   M quickstart/tests/helpers.py
   M quickstart/tests/models/__init__.py
   M quickstart/tests/models/test_charms.py
   M quickstart/tests/models/test_envs.py
   M quickstart/tests/test_app.py
   M quickstart/tests/test_juju.py
   M quickstart/tests/test_manage.py
   A quickstart/tests/test_serializers.py
   M quickstart/tests/test_utils.py
   M quickstart/utils.py

« Back to merge proposal