-
Notifications
You must be signed in to change notification settings - Fork 624
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
Introducing ruff
as linter and formatter (basic setup)
#4223
Conversation
ruff
as linter and formatterruff
as linter and formatter (basic setup)
Note to reviewers Since there are a lot of formatting changes, here are the actual changed files 7a6052e:
For everyone who wants to see it working:
Also, the idea is that we add more rules to ruff linter in separate PRs since there are a lot of fixes to do (maybe create a separate issue for that) . e.g., select = [
"I", # isort
"F", # pyflakes
"E", # pycodestyle errors
"W", # pycodestyle warnings
"N", # pep8-naming
"C90", # mccabe
"RUF", # Ruff-specific rules
"UP", # pyupgrade
"ERA", # eradicate
"PLC", # pylint - convention
"PLE", # pylint - error
"PLW", # pylint - warning
"A", # flake8-builtins
"B", # flake8-bugbear
"BLE", # flake8-blind-except
"C4", # flake8-comprehensions
"Q", # flake8-quotes
"G", # flake8-logging-format
"ICN", # flake8-import-conventions
"ISC", # flake8-implicit-str-concat
"PIE", # flake8-pie
"PTH", # flake8-use-pathlib
"RET", # flake8-return
"S", # flake8-bandit
"SIM", # flake8-simplify
"T10", # flake8-debugger
"T20", # flake8-print
"TCH", # flake8-type-checking
"TID", # flake8-tidy-imports
] |
Love this but I am not sure out of the box ruff will sort imports like isort would do (system, third party, first party sections). |
There's a configuration for |
With ruff 0.6.9 if I run
It will not change anything, I may be missing something in this PR that changes the behavior though |
Ohh I see. The thing is, in
In the pyproject.toml file, there's a section for
run becomes:
|
Signed-off-by: emdneto <[email protected]> remove unecessary lint stuff Signed-off-by: emdneto <[email protected]> update pyproject.toml and pre-commit to auto fix lint Signed-off-by: emdneto <[email protected]> add parameter --show-fixes to pre-commit Signed-off-by: emdneto <[email protected]> some fixes to generate.sh in semconv Signed-off-by: emdneto <[email protected]>
Signed-off-by: emdneto <[email protected]>
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.
This is great, thank you!
Signed-off-by: emdneto <[email protected]>
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.
Lgtm, esp how much shorter the tox.ini is now!
One thought. This question on the other PR seems to be why we didn't merge it.
Adding "do not merge label" until we decide for sure on Ruff replacing flake8 and pylint as well.
I think so, with the basic [tool.ruff.lint] for now and more rules later as you mentioned.
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.
This looks awesome! QQ: Any idea what is going on with the spellcheck env?
Description
This PR swaps out black, isort, and flake8 for ruff, which handles all three in one go! 🎉 Based on this #3260 (comment) it seems there's a general agreement we can adopt ruff.
Closes #3895 #3260
Superseds #3822
Why this change?
We had some issues regarding slow lint in CI and locally (#3887, #3847, ), which required us to use a strategy of linting each package individually, leading to multiple lint commands in tox.ini and several lint jobs in CI. At this moment, lint is very slow to run locally (even by using the tox command)
The main benefit I see here is that we are replacing 3 tools and removing a lot of lint commands from tox.ini. Also ruff is very fast and the configuration is pretty easy to follow.
Note
Right now, ruff has different rules than Pylint. So the question is: Can we take the risk of not using Pylint anymore and relying only on Ruff? In this PR, I'll keep Pylint CI jobs until we decide. If we agree to rely only on Ruff, we can drop all Pylint CI jobs.
What’s changed?