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

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

My tears run deep. I didn't realize it would be this messy supporting
both versions. Does it make any sense to create an alternate ValueError
that can be auto typed?

Yay for working on getting the ball rolling. LGTM with notes below.

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

https://codereview.appspot.com/38500043/diff/1/quickstart/__init__.py#newcode30
quickstart/__init__.py:30: return '.'.join(map(unicode, VERSION))
How does this work with py3 when there is no unicode function? I'd seen
things like
https://github.com/bookieio/breadability/blob/master/breadability/_compat.py#L12

Which creates a unicode reference to str for py3 support.

https://codereview.appspot.com/38500043/diff/1/quickstart/tests/models/test_charms.py
File quickstart/tests/models/test_charms.py (right):

https://codereview.appspot.com/38500043/diff/1/quickstart/tests/models/test_charms.py#newcode33
quickstart/tests/models/test_charms.py:33:
charms.parse_url('precise/juju-gui')
I was expecting these to be u'' strings now.

https://codereview.appspot.com/38500043/

« Back to merge proposal