chore(lint): Enable Pyflakes rules across all packages#497
chore(lint): Enable Pyflakes rules across all packages#497ayushiahjolia wants to merge 1 commit into
Conversation
ayushiahjolia
force-pushed
the
enable-pyflakes-lint
branch
from
8dda1c3 to
976823e
Compare
ayushiahjolia
force-pushed
the
enable-pyflakes-lint
branch
from
976823e to
8a0fcc9
Compare
| import boto3 | ||
|
|
||
| from aws_durable_execution_sdk_python.lambda_service import LambdaClient | ||
| from aws_durable_execution_sdk_python.lambda_service import LambdaClient # noqa: F401 |
There was a problem hiding this comment.
# noqa: F401 exempts this line from the unused imports rule, I assume because we want to keep LambdaClient available for users importing this?
How did we decide which imports to exempt from this rule? Just wondering if this PR automatically removed anything that we wanted to keep.
There was a problem hiding this comment.
-
LambdaClient is there as a fail-fast check. It's in the try/except ImportError block so the CLI dies early if the SDK isn't installed, rather than blowing up later.
-
I asked Kiro to update rules in pyproject.toml, which I think enabled rules and removed all unnecessary imports. There was one import that was not directly used in file but was used at runtime, I found it after pushing changes to this PR and added noqa for it (mock-path resolution for the os one in serialization.py).
| [tool.ruff.lint] | ||
| preview = true | ||
| select = ["TID252"] # Enforce absolute imports (ban relative imports) | ||
| select = ["F", "TID252"] # Pyflakes (unused imports, undefined names, etc.) + absolute imports |
There was a problem hiding this comment.
Unused imports in Python might be actually used because the imported symbols may not be used in the current module, instead be used by other modules importing this module
|
d238353 "update ruff config" added: +select = ["TID252"] # Enforce absolute imports (ban relative imports) This is the regression point. Adding TID252 as the sole entry overrode the default and silently switched off F (and the E4/E7/E9 subset). From here, unused imports/vars accumulated undetected. The PR could optionally also restore E4/E7/E9 (the rest of the old default) via select = ["E4","E7","E9","F","TID252"], since those were dropped in the same commit. And a minor pre-existing inconsistency: the testing package sets preview = false while the others set preview = true. Not necessarily in scope for this PR :-) |
4 participants
Issue #, if available: #479
Description of changes:
Enables the Pyflakes (
F) rule category in ruff across all packages. reviously onlyTID252(ban relative imports) was enforced, meaning unused imports, undefined names, and unused variables went undetected.Testing -
ci-checks.shpasses (tests, coverage, typecheck, fmt for all packages)ruff check --select F,TID252reports zero violationsBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.