-
Notifications
You must be signed in to change notification settings - Fork 71
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
fix: Bump pre-commit hooks to fix isort issue #531
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Deploying with Cloudflare Pages
|
aiven-anton
force-pushed
the
fix/broken-isort-poetry-config
branch
4 times, most recently
from
January 30, 2023 14:11
2d27d39
to
ec3a654
Compare
aiven-anton
commented
Jan 30, 2023
Comment on lines
+25
to
+31
import-error, | ||
consider-using-f-string, | ||
use-implicit-booleaness-not-comparison, | ||
unspecified-encoding, | ||
no-name-in-module, | ||
use-list-literal, | ||
use-dict-literal, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import-error
,no-name-in-module
: pylint is no longer installed with all project dependencies, checks like this is better left to mypy + runtime tests we already have in place.consider-using-f-string
,use-list-literal
,use-dict-literal
: Covered by pyupgrade.unspecified-encoding
: Covered by pyupgrade, but with different resolution.use-implicit-booleaness-not-comparison
: I think this is overly opinionated.foo == []
is fine and doesn't need to be outlawed.
aiven-anton
commented
Jan 30, 2023
@@ -18,7 +18,7 @@ | |||
import aiohttp.web | |||
import aiohttp.web_exceptions | |||
import asyncio | |||
import cgi | |||
import cgi # pylint: disable=deprecated-module |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created a separate issue for this: #533
jjaakola-aiven
previously requested changes
Jan 31, 2023
aiven-anton
commented
Jan 31, 2023
CI is broken on main due to this isort issue, the root cause lies in an underlying incompatible change in Poetry. The new version of isort has dropped support for Python 3.7 so we bump CI to run on 3.11 instead (3.7 is EOL later this year anyway). PyCQA/isort#2083 With the upgrade to flake8 6, we ran into an issue with comments in the configuration file. Those comments are moved to their own lines above the config instead, see issue below. PyCQA/flake8#1756 Because of installation issues in the lint step, installation is altered to only installed pre-commit which handles its own dependencies anyway. Pylint is normalized to be installed just like the other pre-commit hooks, the dependency on the environment is removed.
aiven-anton
force-pushed
the
fix/broken-isort-poetry-config
branch
from
January 31, 2023 08:50
ec3a654
to
d3a104b
Compare
jjaakola-aiven
approved these changes
Feb 1, 2023
tvainika
approved these changes
Feb 1, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
About this change - What it does
Apply
pre-commit autoupdate
and bump CI Python to 3.11, to get isort 5.12.0.Fix flake8 config.
Why this way
The fix is opinionated in that it stops running pylint with project dependencies. I considered suggesting to remove usage of pylint entirely, and I still want to suggest that, but that's a discussion we can have separately from fixing CI builds on main.
CI is broken on main due to this isort issue, the root cause lies in an
underlying incompatible change in Poetry. The new version of isort has
dropped support for Python 3.7 so we bump CI to run on 3.11 instead (3.7
is EOL later this year anyway).
PyCQA/isort#2083
With the upgrade to flake8 6, we ran into an issue with comments in the
configuration file. Those comments are moved to their own lines above
the config instead, see issue below.
PyCQA/flake8#1756
Because of installation issues in the lint step, installation is altered
to only installed pre-commit which handles its own dependencies anyway.
Pylint is normalized to be installed just like the other pre-commit
hooks, the dependency on the environment is removed.