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

Add utf-8 encoding to thrift-generated python files #8022

Merged

Conversation

@blorente
Copy link
Contributor

commented Jul 8, 2019

Problem

When you have utf-8 characters in docstrings inside thrift files, these make it to the generated python sources. Those sources should be utf-8 encoded as well.

Solution

Passed the option py:coding=utf-8 to the thrift compiler, as per https://stackoverflow.com/questions/33887910/thrift-add-encoding-language-in-python-generated-files

Result

We now add the "compile utf-8" header to every non-__init__ generated file.

@illicitonion
Copy link
Contributor

left a comment

Works for me! Would be good to wait for @Eric-Arellano or @benjyw's views before merging :)

@Eric-Arellano
Copy link
Contributor

left a comment

Good change. Even if the thrift files will be used by Py3, it is totally safe to still have this header line. Because this is generated code (where verbosity matters less), I think it’s good to always no matter what include the header.

@Eric-Arellano
Copy link
Contributor

left a comment

Thanks! Yay more pathlib :)

@blorente

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2019

Thank you for the suggestions!

@stuhood

stuhood approved these changes Jul 9, 2019

@stuhood stuhood merged commit 25e2cdd into pantsbuild:master Jul 9, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

illicitonion added a commit that referenced this pull request Jul 10, 2019

Add utf-8 encoding to thrift-generated python files (#8022)
### Problem

When you have utf-8 characters in docstrings inside thrift files, these make it to the generated python sources. Those sources should be utf-8 encoded as well.

### Solution

Passed the option `py:coding=utf-8` to the thrift compiler, as per https://stackoverflow.com/questions/33887910/thrift-add-encoding-language-in-python-generated-files

### Result

We now add the "compile utf-8" header to every non-`__init__` generated file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.