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

Disable lint for Python 3 targets #5765

Merged
merged 6 commits into from Apr 30, 2018

Conversation

Projects
None yet
3 participants
@CMLivingston
Copy link
Contributor

CMLivingston commented Apr 27, 2018

Problem

./pants lint fails on targets containing source code that utilizes Python 2-incompatible syntax.

Solution

Disable lint when targeting Python 3. Track long-term solution with #5764.

Result

Python 3 targets utilizing Python 3-specific syntax will be able to pass lint.

Chris Livingston
self.context.log.info('Linting is currently disabled for Python 3 targets.\n '
'See https://github.com/pantsbuild/pants/issues/5764 for '
'long-term solution tracking.')
return 0

This comment has been minimized.

@wisechengyi

wisechengyi Apr 27, 2018

Contributor

empty return should be fine

This comment has been minimized.

@CMLivingston

CMLivingston Apr 27, 2018

Contributor

👍 will add

# Long-term Python 3 linting solution tracked by:
# https://github.com/pantsbuild/pants/issues/5764
intepreter = self.context.products.get_data(PythonInterpreter)
if intepreter and intepreter.version > (3, 0 ,0):

This comment has been minimized.

@jsirois

jsirois Apr 27, 2018

Member

Products should never be None if your task is executing (the upstream task would have failed and your task's execution should short circuit). You should drop the None test and add this:

@classmethod
def prepare(cls, options, round_manager):
round_manager.require_data(PythonInterpreter)
.

Chris Livingston
@jsirois
Copy link
Member

jsirois left a comment

Thanks. Please leave a comment explaining the None leniency is for tests.

Chris Livingston added some commits Apr 27, 2018

@jsirois
Copy link
Member

jsirois left a comment

Thanks Chris. I'll merge in a few hours once this goes green unless someone else beats me to it.

@@ -14,6 +14,7 @@
from pants.option.custom_types import file_option
from pants.task.lint_task_mixin import LintTaskMixin
from pants.task.task import Task
from pex.interpreter import PythonInterpreter

This comment has been minimized.

@jsirois

jsirois Apr 30, 2018

Member

This should be paired with a '3rdparty/python:pex' dependency in contrib/python/src/python/pants/contrib/python/checks/tasks/checkstyle/BUILD.

interpreter = self.context.products.get_data(PythonInterpreter)
# Check interpreter is not 'None' for test cases that do not
# run the python interpreter selection task.
if interpreter and interpreter.version > (3, 0 ,0):

This comment has been minimized.

@jsirois

jsirois Apr 30, 2018

Member

Although this will get everything we care about, >= (3, 0, 0) reads as more obviously correct.

@wisechengyi wisechengyi merged commit 70b284c into pantsbuild:master Apr 30, 2018

1 check passed

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

CMLivingston pushed a commit to CMLivingston/pants that referenced this pull request Apr 30, 2018

jsirois added a commit that referenced this pull request May 1, 2018

wisechengyi added a commit that referenced this pull request May 1, 2018

Disable lint for Python 3 targets (#5765)
### Problem

`./pants lint` fails on targets containing source code that utilizes Python 2-incompatible syntax.

### Solution

Disable lint when targeting Python 3. Track long-term solution with #5764.

### Result

Python 3 targets utilizing Python 3-specific syntax will be able to pass lint.

wisechengyi added a commit that referenced this pull request May 1, 2018

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