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

No longer use string_types #7980

Merged

Conversation

Eric-Arellano
Copy link
Contributor

@Eric-Arellano Eric-Arellano commented Jun 29, 2019

string_types, which is the same as Tuple[bytes, str], is a vestige of Python 2's loose mixing of unicode and byte strings. Python 3 intentionally makes you choose which you want to use, which we should be doing too for more sane code.

While this technically violates the deprecation policy for some APIs, this coincides with the even larger breaking change of no longer supporting Python 2. It is almost guaranteed that users who extend Pants will already need to make changes, so it is helpful to land this API change at the same time that people are modernizing their pants-plugins.

Further, several of the below instances do not make sense to support bytes—they would crash were someone to pass bytes and it is solely happenstance that no one has reported trying. We had only kept the support for bytes due to Python 2's frequent use of byte strings.

@Eric-Arellano
Copy link
Contributor Author

I can't reproduce the integration shard #4 failures either on macOS or Ubuntu VM :/ Going to try to get this working by analyzing the changes and pushing to CI. Pardon that this will result in some email spam.

FYI example error: https://travis-ci.org/pantsbuild/pants/jobs/552230697#L1039.

@Eric-Arellano Eric-Arellano force-pushed the dont-use-string-types branch 2 times, most recently from 4b7d3ab to 5f78352 Compare June 30, 2019 05:12
@Eric-Arellano Eric-Arellano changed the title WIP: No longer use string_types No longer use string_types Jun 30, 2019
@Eric-Arellano
Copy link
Contributor Author

So turns out I just needed to wipe the Travis cache.. ready for review now.

src/python/pants/backend/jvm/artifact.py Outdated Show resolved Hide resolved
src/python/pants/option/config.py Outdated Show resolved Hide resolved
@Eric-Arellano Eric-Arellano merged commit 0728f73 into pantsbuild:master Jul 1, 2019
@Eric-Arellano Eric-Arellano deleted the dont-use-string-types branch July 1, 2019 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants