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

Port option/ to python3 #6117

Merged
merged 4 commits into from
Jul 17, 2018
Merged

Conversation

ghthor
Copy link
Contributor

@ghthor 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.

Copy link
Sponsor Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

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

Thanks!

@stuhood
Copy link
Sponsor 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
Copy link
Contributor Author

ghthor commented Jul 14, 2018

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

@ghthor
Copy link
Contributor Author

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
Copy link
Contributor

@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.

Copy link
Sponsor Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

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

Thanks!

@stuhood stuhood merged commit bf84716 into pantsbuild:master Jul 17, 2018
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.

3 participants