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

Port option/ to python3 #6117

Merged
merged 4 commits into from Jul 17, 2018

Conversation

Projects
None yet
3 participants
@ghthor
Copy link
Contributor

ghthor commented Jul 13, 2018

Followup for #6062. Almost all automated changes, the the exception of the change from using map() to a for loop.

@stuhood
Copy link
Member

stuhood left a comment

Thanks!

@stuhood

This comment has been minimized.

Copy link
Member

stuhood commented Jul 13, 2018

@ghthor : Looks like there is a lint issue: recommend using build-support/bin/pre-commit.sh before pushing (or installing them as commit hooks with build-support/bin/setup.sh and then skipping with git commit -n .. if need be)

@ghthor

This comment has been minimized.

Copy link
Contributor

ghthor commented Jul 14, 2018

@stuhood noted. I'll do that and update.

@ghthor

This comment has been minimized.

Copy link
Contributor

ghthor commented Jul 16, 2018

Ok, fixed the linting issues. Looks like a test failure[1][2] due to underlying type changes used for comparisons. @Eric-Arellano Does this make sense within the context of what's happening?

[1] https://travis-ci.org/pantsbuild/pants/jobs/404468884#L5273
[2] https://travis-ci.org/pantsbuild/pants/jobs/404468885#L5120

@Eric-Arellano

This comment has been minimized.

Copy link
Contributor

Eric-Arellano commented Jul 16, 2018

@ghthor you can fix the test by adding str to the import on line 9 of options.py.

The reason that test is failing is that test_options.py is using the backported str, whereas options.py is still using the Python2 version of str, so the types aren't matching up correctly.

@ghthor ghthor force-pushed the ghthor:python3-port-option branch from 2f9fe76 to 8aa09d6 Jul 16, 2018

@stuhood
Copy link
Member

stuhood left a comment

Thanks!

@stuhood stuhood merged commit bf84716 into pantsbuild:master Jul 17, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment