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

Enable many more Ruff linter rules #214

Merged
merged 1 commit into from
Feb 6, 2025
Merged

Enable many more Ruff linter rules #214

merged 1 commit into from
Feb 6, 2025

Conversation

seanh
Copy link
Contributor

@seanh seanh commented Jan 28, 2025

...and switch from an opt-in to an opt-out strategy where we enable ALL
rules and then disable certain specific rules or sets of rules. This
means that when new versions of Ruff come out new rules will be enabled
by default.

Slack discussion: https://hypothes-is.slack.com/archives/C1MA4E9B9/p1737635626518379

Draft PRs to apply this to apps:

@seanh seanh requested a review from marcospri January 28, 2025 14:25
@@ -153,10 +113,20 @@ ignore = [
"PLR0913", # Too many arguments. Tests often have lots of arguments.
"PLR0917", # Too many positional arguments. Tests often have lots of arguments.
"PLR0904", # Too many public methods. Test classes often have lots of test methods.
"S101", # Use of `assert` detected.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We want to allow assert's in tests, obviously.

Comment on lines +124 to +129
"{{ cookiecutter.package_name }}/migrations/*" = [
"INP001",
]
"bin/*" = [
"INP001",
]
Copy link
Contributor Author

@seanh seanh Jan 28, 2025

Choose a reason for hiding this comment

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

Missing __init__.py files, which apparently can cause tools like coverage and pytest to silently skip these directories: https://pypi.org/project/flake8-no-pep420/. I don't think we want to add these to migrations/ and bin/?

@seanh seanh force-pushed the enable-ruff-rules branch from 3ac3bb2 to de3e999 Compare January 28, 2025 14:43
Copy link
Member

@marcospri marcospri left a comment

Choose a reason for hiding this comment

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

We might have to tweak the list as we are more familiar with them all but this looks like a pretty good start.

...and switch from an opt-in to an opt-out strategy where we enable ALL
rules and then disable certain specific rules or sets of rules. This
means that when new versions of Ruff come out new rules will be enabled
by default.

Slack discussion: https://hypothes-is.slack.com/archives/C1MA4E9B9/p1737635626518379
@seanh seanh force-pushed the enable-ruff-rules branch from de3e999 to 6afb0c6 Compare February 6, 2025 11:23
@seanh
Copy link
Contributor Author

seanh commented Feb 6, 2025

Rebased

@seanh seanh merged commit d8b1734 into main Feb 6, 2025
4 checks passed
@seanh seanh deleted the enable-ruff-rules branch February 6, 2025 14:53
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.

2 participants