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

Upgrade linter versions to the newest versions that still install under both Python 2 and Python 3.7 #444

Merged
merged 4 commits into from Aug 2, 2019

Conversation

obi1kenobi
Copy link
Contributor

We picked up some bugfixes along the way, so I fixed the lint they detected, and also seemingly a bug in pylint, which I reported: pylint-dev/pylint#3039

@obi1kenobi
Copy link
Contributor Author

We run linting under Python 3.6, but unfortunately the way that pipenv is currently set up requires the linters to successfully install under all environments, and pylint==1.9.5 is marked as not supporting Python 3.7. Similarly, pylint>=2 are Python 3-only, so we have to use pylint==1.9.4 as the closest version that isn't blacklisting Python 3.7. I suspect that version doesn't actually support 3.7, but since the development environment (and by extension the lint environment) is using 3.6, that doesn't really matter here.

TL;DR: My solution is ugly, but it works well enough to not be worth spending additional time on at the moment.

isort = ">=4.3.4,<5"
parameterized = ">=0.6.1,<1"
pydocstyle = ">=2.1.1,<3"
pylint = "==1.9.4" # v1.9.5 is marked as "python_requires < 3.7", update this after we're Py3+ only
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be "python_requires > 3.7" ? (That being the reason why it doesn't work with python2.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. pylint>=2 is python_requires >= 3.3, and pylint==1.9.5 is python_requires < 3.7 -- it explicitly does not support Python 3.7 and is 3.6-and-below only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the PR title to clarify.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok got it. Misunderstood what this meant.

@obi1kenobi obi1kenobi changed the title Upgrade linter versions to the newest versions that still install under Python 2 Upgrade linter versions to the newest versions that still install under both Python 2 and Python 3.7 Aug 2, 2019
@pmantica1 pmantica1 merged commit 1edf355 into master Aug 2, 2019
@pmantica1 pmantica1 deleted the bump_linter_versions branch August 2, 2019 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants