Replace linters and formatters with ruff#1275
Merged
mulkieran merged 8 commits intostratis-storage:masterfrom May 1, 2026
Merged
Replace linters and formatters with ruff#1275mulkieran merged 8 commits intostratis-storage:masterfrom
mulkieran merged 8 commits intostratis-storage:masterfrom
Conversation
mulkieran
force-pushed
the
replace-linters-and-formatters-with-ruff
branch
2 times, most recently
from
95d86e3 to
f321692
Compare
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThe PR migrates the project's code quality toolchain from separate tools ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/stratis_cli/_errors.py (1)
46-50:⚠️ Potential issue | 🟠 MajorAdd
# noqa: PLW0231suppression to the__init__method.Ruff's Pylint rule selection (configured as
select = ["I", "PL"]in pyproject.toml) enables PLW0231, which flagssuper().__init__()not being called. SinceStratisCliNoDeviceSizeChangeError.__init__intentionally skips this call (the class only provides a custom error message via__str__), the suppression is required to pass linting.♻️ Example fix
- def __init__(self): + def __init__(self): # noqa: PLW0231🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stratis_cli/_errors.py` around lines 46 - 50, Add the Ruff Pylint suppression to the StratisCliNoDeviceSizeChangeError.__init__ method by appending "# noqa: PLW0231" to the method definition (so the linter knows the missing super().__init__ call is intentional); ensure the suppression is placed on the def line for __init__ in class StratisCliNoDeviceSizeChangeError so the linter ignores PLW0231 for that method.src/stratis_cli/_actions/_utils.py (1)
55-217:⚠️ Potential issue | 🟠 MajorRestore the removed Ruff suppressions on these classes and exception handler.
The suppressions were removed in the recent commit, but the classes still only define
__init__and (forEncryptionInfo) a single additional methodconsistent(), which triggers Ruff'sPLR0903(too-few-public-methods). Theget_passfunction still has a bareexcept Exception:handler, which triggersPLW0703(broad-exception-caught). Withselect = ["I", "PL"]in the Ruff configuration, these suppressions must be restored either as# noqa: PLR0903,# noqa: PLW0703comments, or addressed by restructuring these classes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stratis_cli/_actions/_utils.py` around lines 55 - 217, Restore the Ruff suppressions by re-adding the appropriate noqa comments: add "# noqa: PLR0903" to the class definitions for EncryptionInfo, EncryptionInfoClevis, EncryptionInfoKeyDescription, and Device (these classes intentionally only define __init__ and a minimal method), and add "# noqa: PLR0903" where appropriate if any other small data-holder classes were affected (e.g., PoolFeature if needed); also restore "# noqa: PLW0703" on the broad except in get_pass (the bare "except Exception:" handler) so Ruff no longer flags PLW0703. Ensure the comments are placed on the same line as the class or except statement so they persist as suppressions for the identified symbols (EncryptionInfo, EncryptionInfoClevis, EncryptionInfoKeyDescription, Device, and get_pass).
🧹 Nitpick comments (2)
pyproject.toml (1)
14-14: Consider broadening Ruff rule selection to include baseline correctness checks.At Line 14,
select = ["I", "PL"]restricts linting to only import sorting and pylint rules, which disables Ruff'sF(Pyflakes) andE(pycodestyle) rules. While Pyright handles type checking, it does not fully replace Pyflakes coverage for issues like undefined names and unused imports. Since Makefile invokes bareruff checkwith no CLI-level--selectoverrides, this narrow scope is the effective lint configuration.Suggested config adjustment
[tool.ruff.lint] -select = ["I", "PL"] +select = ["E4", "E7", "E9", "F", "I", "PL"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pyproject.toml` at line 14, The current Ruff configuration restricts rules via the setting 'select = ["I", "PL"]', which disables important Pyflakes (`F`) and pycodestyle (`E`) checks; update the pyproject.toml Ruff configuration to include at least "F" and "E" (or remove the restrictive select entirely) so bare `ruff check` invoked by the Makefile runs baseline correctness checks (undefined names, unused imports, style errors) in addition to import sorting and pylint rules.tests/integration/pool/test_bind.py (1)
27-27: Derive the loop bound from the new constant.The new
_MAX_LUKS_TOKEN_SLOTS_ALLOWED_TO_BE_USEDstill leavesrange(15)hard-coded, so this test can drift if the slot limit ever changes. Consider deriving the iteration bound from the constant as well.[details]
♻️ Suggested refactor
- for index in range(15): + for index in range(_MAX_LUKS_TOKEN_SLOTS_ALLOWED_TO_BE_USED + 1):Also applies to: 170-184
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/pool/test_bind.py` at line 27, The loop bound is still hard-coded as range(15) while the new constant _MAX_LUKS_TOKEN_SLOTS_ALLOWED_TO_BE_USED governs the intended limit; replace range(15) with range(_MAX_LUKS_TOKEN_SLOTS_ALLOWED_TO_BE_USED + 1) to preserve current behavior (or range(_MAX_LUKS_TOKEN_SLOTS_ALLOWED_TO_BE_USED) if you intended exclusive upper bound) in the occurrences in tests/integration/pool/test_bind.py (update both the range(15) at line ~27 and the one in the 170-184 block) so the test derives its iteration count from _MAX_LUKS_TOKEN_SLOTS_ALLOWED_TO_BE_USED.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Around line 8-14: Update the Makefile's fmt target so import-sorting runs
before formatting: modify the fmt recipe (target name "fmt") to first run "ruff
check --fix --select I" to apply isort-style fixes, then run "ruff format"; also
update the fmt-ci target if desired (currently "fmt-ci" runs "ruff format
--check") to include "ruff check --select I --check" behavior for CI consistency
by invoking "ruff check --select I --check" (or equivalent) before the format
check.
- Around line 4-6: The lint target removed explicit coverage for extensionless
entrypoints; update the Makefile's lint target (the "lint" recipe that currently
runs "ruff check") to include the extensionless script explicitly by invoking
ruff on "bin/stratis" in addition to the directory scan (e.g., run ruff check
against bin/stratis and the project paths) so that the extensionless entrypoint
"bin/stratis" is linted, then leave the existing pyright invocation as-is.
---
Outside diff comments:
In `@src/stratis_cli/_actions/_utils.py`:
- Around line 55-217: Restore the Ruff suppressions by re-adding the appropriate
noqa comments: add "# noqa: PLR0903" to the class definitions for
EncryptionInfo, EncryptionInfoClevis, EncryptionInfoKeyDescription, and Device
(these classes intentionally only define __init__ and a minimal method), and add
"# noqa: PLR0903" where appropriate if any other small data-holder classes were
affected (e.g., PoolFeature if needed); also restore "# noqa: PLW0703" on the
broad except in get_pass (the bare "except Exception:" handler) so Ruff no
longer flags PLW0703. Ensure the comments are placed on the same line as the
class or except statement so they persist as suppressions for the identified
symbols (EncryptionInfo, EncryptionInfoClevis, EncryptionInfoKeyDescription,
Device, and get_pass).
In `@src/stratis_cli/_errors.py`:
- Around line 46-50: Add the Ruff Pylint suppression to the
StratisCliNoDeviceSizeChangeError.__init__ method by appending "# noqa: PLW0231"
to the method definition (so the linter knows the missing super().__init__ call
is intentional); ensure the suppression is placed on the def line for __init__
in class StratisCliNoDeviceSizeChangeError so the linter ignores PLW0231 for
that method.
---
Nitpick comments:
In `@pyproject.toml`:
- Line 14: The current Ruff configuration restricts rules via the setting
'select = ["I", "PL"]', which disables important Pyflakes (`F`) and pycodestyle
(`E`) checks; update the pyproject.toml Ruff configuration to include at least
"F" and "E" (or remove the restrictive select entirely) so bare `ruff check`
invoked by the Makefile runs baseline correctness checks (undefined names,
unused imports, style errors) in addition to import sorting and pylint rules.
In `@tests/integration/pool/test_bind.py`:
- Line 27: The loop bound is still hard-coded as range(15) while the new
constant _MAX_LUKS_TOKEN_SLOTS_ALLOWED_TO_BE_USED governs the intended limit;
replace range(15) with range(_MAX_LUKS_TOKEN_SLOTS_ALLOWED_TO_BE_USED + 1) to
preserve current behavior (or range(_MAX_LUKS_TOKEN_SLOTS_ALLOWED_TO_BE_USED) if
you intended exclusive upper bound) in the occurrences in
tests/integration/pool/test_bind.py (update both the range(15) at line ~27 and
the one in the 170-184 block) so the test derives its iteration count from
_MAX_LUKS_TOKEN_SLOTS_ALLOWED_TO_BE_USED.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1337ac38-749a-4e45-a08e-0d4e04095dc8
📒 Files selected for processing (55)
.github/workflows/main.yml.isort.cfgMakefilepyproject.tomlsrc/stratis_cli/__init__.pysrc/stratis_cli/_actions/_bind.pysrc/stratis_cli/_actions/_connection.pysrc/stratis_cli/_actions/_constants.pysrc/stratis_cli/_actions/_crypt.pysrc/stratis_cli/_actions/_data.pysrc/stratis_cli/_actions/_debug.pysrc/stratis_cli/_actions/_list_filesystem.pysrc/stratis_cli/_actions/_list_pool.pysrc/stratis_cli/_actions/_logical.pysrc/stratis_cli/_actions/_physical.pysrc/stratis_cli/_actions/_pool.pysrc/stratis_cli/_actions/_stratis.pysrc/stratis_cli/_actions/_stratisd_version.pysrc/stratis_cli/_actions/_top.pysrc/stratis_cli/_actions/_utils.pysrc/stratis_cli/_alerts.pysrc/stratis_cli/_constants.pysrc/stratis_cli/_error_reporting.pysrc/stratis_cli/_errors.pysrc/stratis_cli/_exit.pysrc/stratis_cli/_parser/__init__.pysrc/stratis_cli/_parser/_debug.pysrc/stratis_cli/_parser/_encryption.pysrc/stratis_cli/_parser/_logical.pysrc/stratis_cli/_parser/_parser.pysrc/stratis_cli/_parser/_physical.pysrc/stratis_cli/_parser/_pool.pysrc/stratis_cli/_parser/_shared.pysrc/stratis_cli/_stratisd_constants.pytests/_misc.pytests/integration/_keyutils.pytests/integration/key/test_set.pytests/integration/logical/test_cancel_revert.pytests/integration/logical/test_debug.pytests/integration/logical/test_list.pytests/integration/logical/test_schedule_revert.pytests/integration/physical/test_debug.pytests/integration/physical/test_list.pytests/integration/pool/test_add.pytests/integration/pool/test_bind.pytests/integration/pool/test_create.pytests/integration/pool/test_debug.pytests/integration/pool/test_encryption.pytests/integration/pool/test_explain.pytests/integration/pool/test_init_cache.pytests/integration/pool/test_list.pytests/integration/pool/test_stop.pytests/integration/test_parser.pytests/integration/test_stratis.pytests/unit/test_constants.py
💤 Files with no reviewable changes (3)
- tests/integration/_keyutils.py
- src/stratis_cli/_actions/_connection.py
- .isort.cfg
mulkieran
force-pushed
the
replace-linters-and-formatters-with-ruff
branch
2 times, most recently
from
1ada506 to
8708bd4
Compare
Signed-off-by: mulhern <amulhern@redhat.com>
Signed-off-by: mulhern <amulhern@redhat.com>
Signed-off-by: mulhern <amulhern@redhat.com>
Signed-off-by: mulhern <amulhern@redhat.com>
Signed-off-by: mulhern <amulhern@redhat.com>
ruff and pyright do just as well without PYTHONPATH specified. Signed-off-by: mulhern <amulhern@redhat.com>
mulkieran
force-pushed
the
replace-linters-and-formatters-with-ruff
branch
from
8708bd4 to
5a35d69
Compare
jbaublitz
approved these changes
Apr 29, 2026
The placement of the noqa annotations was blocking other transformations. Signed-off-by: mulhern <amulhern@redhat.com>
Signed-off-by: mulhern <amulhern@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related stratis-storage/project#64