Skip to content

Lint: Use Ruff for all formatting in pre-commit #133123

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

AA-Turner
Copy link
Member

It doesn't make that much sense to use two linters in our source tree. This switches Tools/jit and Tools/build/check-warnings.py to use Ruff for their formatting, which has the side benefit of slightly speeding up pre-commit and the lint CI job.

A

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Seems fine to me, but please get sign-off from the people maintaining these directories (I am not!)

@@ -21,9 +21,7 @@ def _async_cache(f: _C[_P, _R]) -> _C[_P, _R]:
lock = asyncio.Lock()

@functools.wraps(f)
async def wrapper(
*args: _P.args, **kwargs: _P.kwargs # pylint: disable = no-member
Copy link
Member

Choose a reason for hiding this comment

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

did you mean to get rid of various pylint suppression comments in this PR? I assume it was there because it was useful to the JIT developers working on this code

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, I'd meant to add them back after reformatting.

cc @brandtbucher to check that the comments are still useful (we don't run pylint in CI, currently)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants