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

fix: Bump pre-commit hooks to fix isort issue #531

Merged
merged 1 commit into from Feb 1, 2023

Conversation

aiven-anton
Copy link
Contributor

@aiven-anton aiven-anton commented Jan 30, 2023

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.

@cloudflare-pages
Copy link

cloudflare-pages bot commented Jan 30, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: d3a104b
Status: ✅  Deploy successful!
Preview URL: https://e5a65a0b.karapace.pages.dev
Branch Preview URL: https://fix-broken-isort-poetry-conf.karapace.pages.dev

View logs

@aiven-anton aiven-anton force-pushed the fix/broken-isort-poetry-config branch 4 times, most recently from 2d27d39 to ec3a654 Compare January 30, 2023 14:11
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,
Copy link
Contributor Author

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 aiven-anton marked this pull request as ready for review January 30, 2023 14:13
@aiven-anton aiven-anton requested review from a team as code owners January 30, 2023 14:13
@@ -18,7 +18,7 @@
import aiohttp.web
import aiohttp.web_exceptions
import asyncio
import cgi
import cgi # pylint: disable=deprecated-module
Copy link
Contributor Author

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

karapace/serialization.py Outdated Show resolved Hide resolved
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.
@tvainika tvainika merged commit 2e58736 into main Feb 1, 2023
@tvainika tvainika deleted the fix/broken-isort-poetry-config branch February 1, 2023 11:57
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.

None yet

3 participants