Code review comment for lp:~benji/landscape-client/try-unicode

Revision history for this message
Adam Collard (adam-collard) wrote :

This branch makes me super uncomfortable. I'm doubling down on my "this feels a bit dangerous". We are changing the code without touching the tests: which means that our tests are using bytes (AKA Python2's str) but our real code is using unicode (AKA Python3's str).

The code in landscape-client was not written with the expectation that string literals are Unicode. This doesn't just affect the on-the-wire format (as Free mentions above) but all interactions with the filesystem, sub-processes, etc.

I think we have to bite the bullet and just accept that we have to take the time to identify what is textual data and what is binary (and mark up literals with u"" and b"" as necessary)

review: Disapprove

« Back to merge proposal