Skip to content

Switch to Ruff & set up pyright (opt-in)#16

Merged
sergei-maertens merged 6 commits into
mainfrom
chore/switch-to-ruff
Apr 29, 2025
Merged

Switch to Ruff & set up pyright (opt-in)#16
sergei-maertens merged 6 commits into
mainfrom
chore/switch-to-ruff

Conversation

@sergei-maertens
Copy link
Copy Markdown
Member

After the consensus among the team :)

* Swap out isort/black/flake8 for ruff
* Add placeholder to suggest type checking
* Updated the used github actions to their latest versions
* Updated default python/django support matrix (include Django 5.2,
  start from Python 3.12)
Comment on lines -10 to -11
paths:
- '**.py'
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removed this as I've had plenty of cases where config changes lead to the CI checks not running and the next one doing code changes has to deal with the broken config from the previous contribution.

Ruff is fast enough, there's no reason to not just always run these checks.

Comment thread pyproject.toml
Comment on lines +103 to +113
extend-select = [
"UP", # pyupgrade
"DJ", # django
"LOG", # logging
"G",
"I", # isort
"E", # pycodestyle
"F", # pyflakes
"PERF",# perflint
"B006",# flake8-bugbear
"B010",
"B904",
]
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

picked the config from OIP/django-upgrade-check, @swrichards is working on a nitpick solution to keep those in sync/centralized

Comment thread pyproject.toml Outdated
Comment thread pyproject.toml
Comment thread tox.ini Outdated
@swrichards
Copy link
Copy Markdown

After the consensus among the team :)

In general I don't see any compelling reasons against going all-in on ruff (beyond the well-known VC rug-pull concerns, which it seems to me are adequately addressed due to the licensing structure), and specifically for the starter template I think we can be a bit more ambitious as to the scope of the rules.

The guiding principle I think would be to identify rules that don't fall into the "reasonable people can disagree" column: for instance, the SIM (simplify) class of rules are full of things that you can legitimately think are not helpful (e.g. aggressively consolidating conditions to the point of un-readability).

I think this ruleset is fairly balanced in this regard. On the style end of the spectrum the Django, isort and python-next rules make sense to be me as style-focused but non-optional (Django and isort for cross-project readability, python-next because adapting to the latest language idiom should be a standing goal). Bugbear, perf, logging and the like point mainly to things which are most of the time undesirable patterns and potential errors, to the extent that making them mandatory seems sensible.

It might well be the case that the rule classes include individual rules that are cumbersome or unhelpful, but it's hard to get a sense of which without unleashing this on a large code base. I think it would not be a problem if individual projects have opt outs for individual rules (and any tooling around this would permit that) though, so if this is encountered, the exclude option is only a few lines away. It would make sense to take stock at some point to see which rules fall within the exclusion consensus.

Note that bringing existing projects up to scratch with this list is no small undertaking, but that's a separate discussion.

* Added Ruff linter configuration
* Replaced isort config with Ruff isort plugin
* Removed flake8 config
* Added pyright config (opt-in)
* Replaced black, isort and flake8 envs with a single ruff env
* Ruff outputs in a format that can be picked up by Github actions for
  actual code annotations.
* Added env for type checking with pyright, deactivated by default.
It's replaced by the raw make calls in tox.ini which provide better to
parse output anyway.
Comment thread docs/check_sphinx.py-tpl
@sergei-maertens sergei-maertens merged commit 93ca7ca into main Apr 29, 2025
1 check passed
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.

3 participants