-
Notifications
You must be signed in to change notification settings - Fork 57
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
introduce Ruff as linter #345
Conversation
ignore = "B006,B008,W503" | ||
max-line-length = 160 | ||
exclude = "venv/*,tox/*" | ||
ignore = ["B006", "B008"] |
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.
W503
is not currently supported in ruff
and it will not be implemented.
ref: astral-sh/ruff#4125
Is anyone who want to ignore this error?
test/test_redis_lock.py
Outdated
@@ -1,4 +1,4 @@ | |||
import random | |||
import random # noqa: I001 |
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 noqa
is needed for line 8.
If I remove this noqa
the line will fixed as following format by ruff
from gokart.redis_lock import (
RedisClient,
RedisParams,
make_redis_key,
make_redis_params,
wrap_with_dump_lock,
wrap_with_load_lock,
wrap_with_remove_lock,
wrap_with_run_lock,
)
To minimize diff in this PR I added it but it needs to discuss with this issue.
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.
[imo] +1 to ruff
's one, and +10 to accept default behaviors.
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.
applied! e834ad0
@hirosassa Great! Can you compare exection time? |
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 👍
@yokomotod In my local PC, using
|
@yokomotod All of your suggestions are fixed! plz check. |
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!
What I did in this PR
related to #344
I introduce
ruff
as linter.I replaced
flake8
andisort
to simplify the configuration.What I did not do
Replacing
yapf
withruff
is next task.