-
-
Notifications
You must be signed in to change notification settings - Fork 547
Replace linters with ruff #3145
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
Replace linters with ruff #3145
Conversation
Migrated from flake8, black, and isort to Ruff for faster and unified Python linting and formatting. Changes: - Replaced flake8, black, and isort with Ruff in pre-commit hooks - Updated GitHub Actions workflow to use Ruff - Migrated all linter configurations to pyproject.toml - Updated README badges to show Ruff instead of black/isort - Updated PR template to reference Ruff instead of old linters - Deleted .flake8 configuration file (migrated to pyproject.toml) - Updated test requirements to use ruff==0.8.6 Benefits: - 10-100x faster than existing tools - Single tool replaces 3 separate linters - Native flake8-django support - Actively maintained and modern
mlodic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the contribution, this is very useful.
I enabled the CI, can you please fix it?
Also, to complete this task, we would also need to adjust the official documentation too: can you update the references to the old linters [here] (https://github.com/intelowlproject/docs/blob/main/docs/IntelOwl/contribute.md) in a separate PR?
| "B", # flake8-bugbear | ||
| "C4", # flake8-comprehensions | ||
| "DJ", # flake8-django | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this specific set has been chosen over all the possibilities?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The selected ruleset is intentionally conservative and maps closely to the linters we were already using (flake8 core, flake8-django, isort), with a few widely adopted extensions like bugbear and comprehensions that catch real issues with low noise
Core Rules (Direct Replacements)
E,W,F- Direct replacement for flake8 coreI- Direct replacement for isortDJ- Replacement for unmaintained flake8-django (primary reason for this migration)
|
@kami922 please provide feedback whether you are willing to complete this PR or not. The contribution rules would have required that you asked to be assigned to this task but you didn't, plus PRs can stay stale for longer than a week so it is important that you confirm your willingness to complete this in respect of the contribution rules. Thanks |
|
@mlodic Hello yes i am working on it sorry about the assignment issue i will fix it and send request changes in 3-4 hours in the mean time can you please assign this to me |
|
sure, thanks for the fast feedback, really appreciated |
W503 is deprecated in Ruff and handled automatically by the formatter. This was causing CI to fail with 'Unknown rule selector: W503'.
|
@mlodic Hello good day to you. I have created 3 files
|
|
@mlodic Regarding the updation of docs can you please help me clear me confusion |
|
About documenting the changes made, sure, please insert those files in the .github directory in a specific new folder About second question, yes, please fork the docs repo. No need of creating an additional issue to track there, you can reference the issue on this repo there |
|
@mlodic I have a question regarding the scope of this migration: currently ruff replaces flake8 + isort for linting only, while Black is kept for formatting. Because Using ruff format instead of Black would reformat 298 files due to minor formatting differences between the two tools. Options:
Which approach do you prefer? I can do either - just wanted to confirm before making such a large formatting change across the codebase. |
- Ruff replaces flake8 + isort for linting - Black kept for formatting (avoids reformatting 298 files) - Fixed auto-fixable lint errors - Reduced Ruff rules to E, W, F, I only
|
thanks for asking. I think that we can separate the 2 things in 2 different PRs so that it is easier to keep track and, eventually, revert the changes. |
|
@mlodic can you please check if this okay and After it is merged I shall update the documentation https://github.com/intelowlproject/docs/blob/main/docs/IntelOwl/contribute.md referencing it then go towards full migration (Ruff replaces Black and 298 files reformatting). |
requirements/test-requirements.txt
Outdated
| @@ -1,5 +1,4 @@ | |||
| flake8==7.1.1 | |||
| ruff==0.8.6 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any particular reason to use an old version of ruff ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's good catch, we must use the most recent one
.pre-commit-config.yaml
Outdated
| @@ -1,26 +1,14 @@ | |||
| repos: | |||
| - repo: https://github.com/astral-sh/ruff-pre-commit | |||
| rev: v0.8.6 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any particular reason to use an old version ?
|
Also, we recently completed the migration to Ruff in our other project Greedybear. |
logs.txt
Outdated
| jan 1 thursday | ||
| setup | ||
| View logs: docker logs -f intelowl_uwsgi | ||
| Stop all services: ./start prod down |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this, save it into somewhere else locally...
- Updated ruff from 0.8.6 to 0.14.10 in requirements/test-requirements.txt - Updated ruff pre-commit hook from v0.8.6 to v0.14.10 in .pre-commit-config.yaml - Removed logs.txt (local file, should not be in repo)
|
@mlodic @fgibertoni sorry for silly mistake i hope its fixed now in new commit let me know whats left to close this PR, In other PR I have to implement |
|
Thanks for the fast response and fix @kami922. |
|
thanks! @kami922 looking forward to your next PR :) |
|
@mlodic Hello i will try to send that PR over the weekend as I got caught up in my final exams. Also just to confirm the next PR will contain full Ruff Migration with above mentioned rules and all formatted files. |
|
yep sure, thank you, no worries |
Description
This PR migrates IntelOwl from using multiple Python linters (flake8, black, and isort) to using Ruff - a modern, fast, all-in-one Python linter and formatter written in Rust.
Fixes #3142
Motivation
As mentioned in issue #3142, flake8-django is no longer actively maintained. Additionally, using three separate tools for linting and formatting increases CI/CD time and complexity. Ruff provides a unified solution that is significantly faster and actively maintained.
Changes Made
.pre-commit-config.yaml.github/workflows/pull_request_automation.yml) to use Ruff for linting and formatting.flake8topyproject.tomlwith Ruff configuration.github/pull_request_template.md) to reference Ruff instead of old linters.flake8configuration file (no longer needed)requirements/test-requirements.txtto useruff==0.8.6instead of flake8, black, and isortRuff Configuration Details
The Ruff configuration in
pyproject.tomlincludes:E,W(pycodestyle)F(pyflakes)I(isort)N(pep8-naming)UP(pyupgrade)B(flake8-bugbear)C4(flake8-comprehensions)DJ(flake8-django) ✨Benefits
✅ 10-100x faster than existing tools
✅ Single tool replaces 3 separate linters
✅ Native flake8-django support (addresses issue #3142)
✅ Actively maintained and modern
✅ Faster CI/CD - reduced linting time
✅ Simpler configuration - everything in
pyproject.tomlType of change
Checklist
developRuff) gave 0 errorsFiles Changed