Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove .decode('utf-8') for python3 support #15

Closed
wants to merge 1 commit into from

Conversation

yusufsyaifudin
Copy link

this PR intended to fix this issue #14

As mentioned by @Rogdham, I think that we should removed .decode('utf-8') because "Begin with Python 3, all string is unicode object.", based recommendation from answer for this question and answer for this question, it should be removed.

note:
I've tested running the test py.test --cov-report= --cov=ssh-audit -v test and python3 -m py.test --cov-report= --cov=ssh-audit -v test for python3 and have no error.

But, double check should be done because I don't know what the reason behind of using .decode('utf-8'), maybe it to avoid strange behavior for special case (?).

@coveralls
Copy link

coveralls commented Oct 16, 2016

Coverage Status

Coverage remained the same at 56.726% when pulling 8507064 on yusufsyaifudin:master into 76509a1 on arthepsy:master.

arthepsy added a commit that referenced this pull request Oct 17, 2016
Add static type checks via mypy (optional static type checker),
Add relevant tests, which could trigger the issue.
@arthepsy arthepsy added the bug label Oct 17, 2016
@arthepsy
Copy link
Owner

Thanks for the work and references. You are right, that this bug got introduced to avoid strange case. But as this part of the code wasn't tested automatically (jet), it opened gates for other bug. For more information, read my comment in #14.

I don't think that it's the correct fix to remove .decode('utf-8'), because the source data is bytes and it should be decoded to get back unicode (Python 2) or str (Python 3).

Therefore, I fixed it a bit differently. Also, I added static type checks via (http://mypy-lang.org/) to be sure of the fix. In any case, can You verify that 6b76e68 fixes the bug?

@arthepsy
Copy link
Owner

This and other encoding/decoding issues are fixed in 1.7.0. Also, multiple of tests added.

@arthepsy arthepsy closed this Oct 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants