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

Switch to ruff for linting #9283

Merged
merged 11 commits into from
Apr 24, 2023
Merged

Switch to ruff for linting #9283

merged 11 commits into from
Apr 24, 2023

Conversation

zanieb
Copy link
Contributor

@zanieb zanieb commented Apr 20, 2023

Because it's the hot new tool and it's way faster than our existing tools.

  • Removes isort/flake8/autoflake8 in favor of ruff
  • Runs ruff --fix
  • Manually fixes remaining ruff errors

This caught a bunch of shadowed test names resulting in missed test coverage or confusing duplication.

Note there's a minor gotcha when using --fix with ORM filters; see astral-sh/ruff#2443 (comment)

I also installed the Ruff VSCode plugin which I would recommend.

I started fixing all these E501 Line too long errors (which are mostly in documentation) but got sick of it and am not sure it improves readability or provides real value. I tried to disable the check for comments so it'd just check the code, but black will fix the code when it's too long so we don't really need to have it on at all.

On main:

❯ ruff check . --statistics
625	E501	[ ] Line too long (100 > 88 characters)
234	F841	[*] Local variable `Child` is assigned to but never used
137	F541	[*] f-string without any placeholders
 88	F401	[*] `.._internal.compatibility.test_experimental.enable_prefect_experimental_test_opt_in_setting` imported but unused
 46	E402	[ ] Module level import not at top of file
 40	E731	[*] Do not assign a `lambda` expression, use a `def`
 26	F811	[ ] Redefinition of unused `AutoEnum` from line 70
 25	E711	[*] Comparison to `None` should be `cond is None`
 19	F821	[ ] Undefined name `Any`
 13	F822	[ ] Undefined name `flow_name` in `__all__`
  9	E712	[*] Comparison to `False` should be `cond is False` or `if not cond:`
  8	F403	[ ] `from .fixtures.api import *` used; unable to detect undefined names
  8	F405	[ ] `Block` may be undefined, or defined from star imports: `.fixtures.api`, `.fixtures.client`, `.fixtures.database`, `.fixtures.docker`, `.fixtures.logging`, `.fixtures.storage`, `prefect.testing.cli`, `prefect.testing.fixtures`
  5	E722	[ ] Do not use bare `except`
  2	E713	[*] Test for membership should be `not in`
  1	E714	[*] Test for object identity should be `is not`
  1	F402	[ ] Import `call` from line 1 shadowed by loop variable
  1	F601	[ ] Dictionary key literal `"thing_one"` repeated

@netlify
Copy link

netlify bot commented Apr 20, 2023

Deploy Preview for prefect-docs-preview ready!

Name Link
🔨 Latest commit 2d078fe
🔍 Latest deploy log https://app.netlify.com/sites/prefect-docs-preview/deploys/6446bdaf91801c0007681dd1
😎 Deploy Preview https://deploy-preview-9283--prefect-docs-preview.netlify.app/contributing/overview
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@zanieb zanieb added the development Tech debt, refactors, CI, tests, and other related work. label Apr 24, 2023
@zanieb zanieb marked this pull request as ready for review April 24, 2023 17:37
@zanieb zanieb requested review from a team and zangell44 as code owners April 24, 2023 17:37
@zanieb zanieb merged commit d54ae14 into main Apr 24, 2023
@zanieb zanieb deleted the ruff branch April 24, 2023 19:20
@zanieb zanieb removed the migration label Apr 24, 2023
asmundo pushed a commit to asmundo/prefect that referenced this pull request May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Tech debt, refactors, CI, tests, and other related work.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants