Skip to content

Add ruff.toml and clean up warnings #519

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

Merged
merged 4 commits into from
Feb 27, 2023

Conversation

nathanchance
Copy link
Member

ruff is a fast Python linter written in Rust. Add a ruff.toml file to turn on several classes of warnings that are useful and fix them up.

Copy link
Member

@nickdesaulniers nickdesaulniers left a comment

Choose a reason for hiding this comment

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

The commit message Fix 'ruff check --fix' for COM812 doesn't really explain what's going on. Got the warning text handy?

@nathanchance
Copy link
Member Author

nathanchance commented Feb 27, 2023

The commit message Fix 'ruff check --fix' for COM812 doesn't really explain what's going on. Got the warning text handy?

Yeah, sorry, that should read Run 'ruff check --fix' for COM812. It basically just complains if each line in a multi-line call or iterable does not end in a comma

generate.py:38:87: COM812 [*] Trailing comma missing

and the --fix just automatically fixes it for us.

@nickdesaulniers
Copy link
Member

The commit message Fix 'ruff check --fix' for COM812 doesn't really explain what's going on. Got the warning text handy?

Yeah, sorry, that should read Run 'ruff check --fix' for COM812. It basically just complains if each line in a multi-line call or iterable does not end in a comma

generate.py:38:87: COM812 [*] Trailing comma missing

and the --fix just automatically fixes it for us.

Can we just disable that check and drop the commit that added trailing commas? They make sense IMO when defining members of an array, so that additions in later commits don't modify the git blame. Seeing them in parameter lists makes my eyes bleed.

@nathanchance
Copy link
Member Author

Sure, I figured it was a useful check that would result in some churn up front and not too much latter but I do not care too much.

This is an opinionated set of warnings from the full list that ruff
supports.

Link: https://beta.ruff.rs/docs/rules/
Signed-off-by: Nathan Chancellor <[email protected]>
Fixes a ruff warning:

  generate.py:25:17: RUF005 [*] Consider `[*trees, 'all']` instead of concatenation

Signed-off-by: Nathan Chancellor <[email protected]>
Fixes new ruff warning:

  check_logs.py:148:13: PLW2901 Outer for loop variable `line` overwritten by inner assignment target

This warning is in pylint as well, but it is not enabled under the
default configuration. Additionally, simplify the check for an empty
line. We can use the walrus operator plus the truthiness of the empty
string instead of assigning and checking the length separately.

Signed-off-by: Nathan Chancellor <[email protected]>
Signed-off-by: Nathan Chancellor <[email protected]>
@nathanchance nathanchance merged commit a39ce70 into ClangBuiltLinux:main Feb 27, 2023
@nathanchance nathanchance deleted the ruff branch February 27, 2023 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants