Bump Ruff to v0.15.18 and address new lint violations#826
Conversation
psf-chronographer
Bot
added
the
bot:chronographer:provided
julianz-
force-pushed
the
bump-ruff-v0.15.18
branch
from
794639b to
2bfeca8
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #826 +/- ##
==========================================
- Coverage 78.19% 75.85% -2.34%
==========================================
Files 41 41
Lines 4788 4788
Branches 547 482 -65
==========================================
- Hits 3744 3632 -112
- Misses 905 1012 +107
- Partials 139 144 +5 |
webknjaz
left a comment
webknjaz
left a comment
There was a problem hiding this comment.
Do you feel strongly about those noqas?
| "PLR6301", # no-self-use # FIXME / noqa | ||
|
|
||
| "PLW0717", # too-many-statements-in-try-clause # FIXME | ||
|
|
| "PLR6104", # non-augmented-assignment # FIXME | ||
| "PLR6301", # no-self-use # FIXME / noqa | ||
|
|
||
| "PLW0717", # too-many-statements-in-try-clause # FIXME |
There was a problem hiding this comment.
Are there many of these? I mostly prefer narrowly scoped ignores when adding new rules.
There was a problem hiding this comment.
There are 8 violations across 6 files. I've moved PLW0717 to per-file ignores (production files + test files) rather than keeping it in the global suppress list — that way the rule stays active everywhere else. Happy to switch to individual # noqa: PLW0717 lines on each violation if you prefer?
| '<socket.socket fd=-1, family=AF_INET6, ' | ||
| 'type=SocketKind.SOCK_STREAM, proto=.:' | ||
| 'pytest.PytestUnraisableExceptionWarning:_pytest.unraisableexception', | ||
| 'ignore:Exception in thread CP Server Thread-:pytest.PytestUnhandledThreadExceptionWarning:_pytest.threadexception', # noqa: LN001 |
There was a problem hiding this comment.
I'd like to keep these multi-line instead of having ignores. Let's go for nested parentheses instead.
| Three further new rules are suppressed pending fixes: too many statements | ||
| in a try clause (PLW0717), non-empty ``__init__`` modules (RUF067), and | ||
| starting a process with a partial executable path (S607) | ||
| -- by :user:`julianz-`. |
There was a problem hiding this comment.
You probably want this
| -- by :user:`julianz-`. | |
| -- by :user:`julianz-` |
There was a problem hiding this comment.
Render's broken here: https://cheroot--826.org.readthedocs.build/en/826/history/#contributor-facing-changes
There was a problem hiding this comment.
Think fixed now hopefully.
- Fix D421: reword property docstrings to noun phrases - Fix ISC004: parenthesize implicit string concatenation in collections; use single strings with # noqa: LN001 where readability is better - Drop now-redundant # noqa: D401 comments on property definitions - Suppress PLW0717, RUF067, S607 pending future fixes
julianz-
force-pushed
the
bump-ruff-v0.15.18
branch
from
2bfeca8 to
323f3b9
Compare
There was a problem hiding this comment.
@julianz- for the future, once you refactor socket handling + TLS, we may be able to drop these warning suppressions, hopefully. Actually, we should try doing so in the respective PRs if they are in place already.
There was a problem hiding this comment.
I removed the socket exception suppressions from #779.
| 'Found error in the error log: ' | ||
| f"message = '{c_msg}', level = '{c_level}'\n" | ||
| f'{c_traceback}', | ||
| f"Found error in the error log: message = '{c_msg}', level = '{c_level}'\n{c_traceback}" # noqa: LN001 |
There was a problem hiding this comment.
Perhaps, this will do?
| f"Found error in the error log: message = '{c_msg}', level = '{c_level}'\n{c_traceback}" # noqa: LN001 | |
| f"Found error in the error log: message = '{c_msg}', " | |
| f"level = '{c_level}'\n{c_traceback}" |
There was a problem hiding this comment.
Let's address the violations instead of suppressing them.
| Three further new rules are suppressed pending fixes: too many statements | ||
| in a try clause (PLW0717), non-empty ``__init__`` modules (RUF067), and | ||
| starting a process with a partial executable path (S607) | ||
|
|
There was a problem hiding this comment.
Either make the byline a part of the sentence (no need for moving it into a separate paragraph w/ a trailing period), or add a trailing period to the previous paragraph and drop it from the byline.
There was a problem hiding this comment.
Not sure what you mean here?
|
Another note: I'm not sure how I feel about marking agents commit authors. I'd rather have a plain text mention or an assisted-by trailer. It's probably good to port the ai policy from pip-tools into this project. |
Suppresses too-many-statements-in-try-clause only in the files that actually trigger it, rather than globally. Test files are also covered. Global suppression masked the rule entirely; per-file ignores leave it active elsewhere and make the FIXMEs more targeted.
julianz-
force-pushed
the
bump-ruff-v0.15.18
branch
from
a9282c0 to
50e147f
Compare
… from test files Replace # noqa: LN001 suppressions with properly split strings wrapped in parentheses to satisfy ISC004. Remove PLW0717 from the test file ignore list — test files have no violations so the suppression was unnecessary.
julianz-
force-pushed
the
bump-ruff-v0.15.18
branch
from
50e147f to
027fabb
Compare
2 participants
Updates Ruff from v0.13.3 to v0.15.18
This requires the following changes:
❓ What kind of change does this PR introduce?
📋 What is the related issue number (starting with
#)Resolves #
❓ What is the current behavior? (You can also link to an open issue here)
❓ What is the new behavior (if this is a feature change)?
📋 Other information:
📋 Contribution checklist:
(If you're a first-timer, check out
[this guide on making great pull requests][making a lovely PR])
the changes have been approved
and description in grammatically correct, complete sentences