search

Scout24 / afp-cli

CLI for the AWS Federation Proxy
Apache License 2.0
13 stars 8 forks source link

Issue fix 43 #46

Closed karolyi closed 8 years ago

karolyi commented 8 years ago

fixing issue #43, utf8 handling is generally a PITA with py2

esc commented 8 years ago

I still haven't tried this out but I don't feel comfortable with the amount of reformatting and beautification. First, it makes the diff in the pull-request quite hard to read and second I do like the way the imports are formatted and spent quite a lot time making them look nice.

esc commented 8 years ago

Maybe in future you could split feature/fix and beautifcation into two pull requests.

esc commented 8 years ago

Anyway, let me get this tried manually today.

karolyi commented 8 years ago

This does not work yet, travis produces absolutely weird errors via the request library which I'm unable to reproduce locally (which means it works locally, not just the tests but afp-cli itself installed and tried out).

We'll use another approach to fix this bug.

karolyi commented 8 years ago

The import beautifications are done by isort.

esc commented 8 years ago

Yes, I know about isort, I basically stopped "beautifying" other peoples code after I watched;

http://pyvideo.org/video/3511/beyond-pep-8-best-practices-for-beautiful-inte

Still the point about splitting beautifcation and bugfix remains.

esc commented 8 years ago

Will close in favour of #47