[Issue #1587] Switch to use ruff instead of flake8 #1626
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chouinar
requested review from
sumiat,
widal001,
andycochran,
acouch,
SammySteiner and
aplybeah
as code owners
github-actions
bot
added
documentation
chouinar
commented
Apr 5, 2024
| @@ -120,7 +120,7 @@ def reload_repl() -> None: | |||
|
|
|||
| try: | |||
| importlib.reload(module) | |||
| except: # noqa: B001 | |||
| except Exception: | |||
There was a problem hiding this comment.
This is the only necessary code change. We used to ignore bare exceptions, but I think I prefer having that check in place. Since it only occurred in a single place within a local development tool and the docs even explicitly state that one reason you don't want to do this is keyboard interrupts, I think its just better to get rid of it:
https://docs.astral.sh/ruff/rules/bare-except/
coilysiren
approved these changes
Apr 15, 2024
chouinar
added a commit
to navapbc/template-application-flask
that referenced
this pull request
Jun 26, 2024
## Ticket Resolves #107 ## Changes Change our linter from flake8 to ruff Python package updates ## Context for reviewers [ruff](https://docs.astral.sh/ruff/) is a linter built in Rust that is both better performing and contains much more options than flake8 (including many of the Flake8 rules). Ruff has a lot of [potential rules](https://docs.astral.sh/ruff/rules/) we could configure, but to keep things simple, I ported over the same rules that we already had configured. If we want to try out any of the other rules, we can follow up with that. For context, enabling all rules causes ~1300 errors, although many of the common ones I see are not issues I think we would want to actually consider (complaining about using `assert` in test functions) Ruff can also serve as a formatter of code, potentially replacing black + isort, but I'll explore using that later as the change is likely much bigger as it would cause a lot of little formatting differences ## Testing Was able to run ruff via `make lint-ruff` and only encountered a few small issues that I fixed. We also switched to using this approach a few months ago in the simpler grants codebase and haven't run into any issues. HHS/simpler-grants-gov#1626 --------- Co-authored-by: nava-platform-bot <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1587
Time to review: 3 mins
Changes proposed
Update our API linting to use ruff instead of flake8
Context for reviewers
ruff is a linter (and formatter although we aren't using that yet) that does what flake8 does, but does it faster and with many more configuration options (not yet used). Conveniently, it even uses the same rules as flake8, so switching just required adjusting where the configuration was defined.
Note that https://pypi.org/project/flake8-mypy/ was something we previously had configured, but has been abandoned for years, and I'm not quite sure what its purpose was (we use mypy - which handles type linting anyways?).
A few of the ignore rules were modified as we were either ignoring things that were removed or that just made sense to no longer ignore.
There are many, many rules that we could add, but I wanted to keep this PR contained to just porting us over, and not reconfiguring things - turning on every rule gives 3k+ errors, mostly in our tests for things we wouldn't care about. We likely will need to have different settings for tests and non-tests if we want to turn on a lot more.
Additionally, while ruff can be used as a formatter, and replace our usage of isort & black potentially, that would cause a fair number of changes at the moment (~25 files, mostly commas, line-length and white space), and I'll leave that as a later follow-up.