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.

Sign up for GitHub

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

Jump to bottom

Update setup.py #221

Closed
wants to merge 2 commits into from
Closed

Update setup.py #221

wants to merge 2 commits into from

Conversation

emielrv
Copy link

@emielrv emielrv commented Nov 13, 2019

Fix windows encoding issue

Summary

  • Fixed the encoding issue in windows of the changelog when pip installing

Issues

@emielrv
Update setup.py
c0e622c 
Fix windows encoding issue
@emielrv emielrv requested a review from a team as a code owner November 13, 2019 16:36
@aliabbasrizvi
Copy link
Contributor

@emielrv thank you for submitting a PR to address this. As a pre-requisite can you please sign our CLA mentioned in the contributing doc: https://github.com/optimizely/python-sdk/blob/master/CONTRIBUTING.md

@emielrv
Copy link
Author

emielrv commented Nov 14, 2019

I signed the CLA

aliabbasrizvi
aliabbasrizvi reviewed Nov 15, 2019
View reviewed changes
setup.py
@@ -20,7 +20,7 @@
with open(os.path.join(here, 'README.md')) as _file:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may have to make the change here as well?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, to be honest I don't know what would be a best practice here. I know only changing it here will solve the problem, since the rest doesn't have these special characters. But maybe it is a best practice to change it everywhere.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the change you made will not work with Python 2.7.

We can try updating the content in CHANGELOG itself.

@aliabbasrizvi
Copy link
Contributor

This was addressed in 3.3.1 release. Closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aliabbasrizvi aliabbasrizvi aliabbasrizvi left review comments

Assignees

@emielrv emielrv

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Pip raises UnicodeDecodeError when installing on windows
2 participants
@emielrv @aliabbasrizvi