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

chore: move to ruff for linting #1120

Merged
merged 2 commits into from Feb 4, 2024

Conversation

tonyandrewmeyer
Copy link
Contributor

This PR replaces flake8, flake8-docstrings, flake8-builtins, isort, autopep8, pep8-naming, pyproject-flake8, and two test_infra unit tests with ruff for linting. Formatting remains unchanged (to be addressed in #1103).

Notes:

  1. Ruff does not yet implement all of the pycodestyle rules, which we checked/fixed with autopep8, see: Implement remaining pycodestyle rules astral-sh/ruff#2402
  2. Ruff implements all Flake8 rules (the "F" rules).
  3. flake8-docstrings is not listed in the Ruff re-implementation list by pydocstyle is there, which flake8-docstrings is based on
  4. Ruff reimplements flake8-builtins (but picks up much more than we were previously, which is odd).
  5. Ruff's isort is "profile='black'".
  6. With flake8 we had "R" rules enabled, but I can't figure out what those were, or what provided them.

Where the linter picks up issues that the old tools did not, handle in one of these ways:

  • Ignore with a noqa: directive if it's a false positive or should otherwise be permanently ignored in that specific case
  • Ignore for a file or group of files (the docs and tests have several of these) where it's something we want to pick up in the core code but not everywhere
  • Ignore with a note to review later when it's likely that there would be too much additional noise in this PR
  • Make the recommended change, when it's small and seems reasonable

#1104 will continue on from this with a few more changes that are minimal and reasonable, and enabling additional rule sets (since they are bundled with ruff, and since ruff is so fast, they are basically free) that I agree with/like the most.

A few outdated noqa: directives have been removed (ruff detects these as well).

Fixes #1102.

Copy link
Collaborator

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

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

This looks great and was easy to review -- thanks!

@benhoyt benhoyt merged commit 5f7079a into canonical:main Feb 4, 2024
28 checks passed
@tonyandrewmeyer tonyandrewmeyer deleted the ruff-linting-minimal-1102 branch February 6, 2024 22:03
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.

Use Ruff linter on the project
2 participants