Replace linters and formatters with ruff#577
Conversation
Remove from Makefile, GitHub Actions, and source code. Signed-off-by: mulhern <amulhern@redhat.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughMigrates the project's linting and formatting toolchain from pylint/black/isort to ruff, updates CI workflows and Makefile accordingly, removes isort configuration, consolidates ruff settings into pyproject.toml, and performs formatting refactors throughout Python scripts while removing now-unnecessary linter suppression comments. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
misc_scripts/thin_metadata_size_viz.py (2)
119-163:⚠️ Potential issue | 🟠 Major
main()violates the RuffPLR0914local-variable limit.With 18 local variables, the function exceeds the default threshold of 5. The code needs to be refactored (e.g., by extracting validation logic or grouping related variables into objects) or the threshold must be adjusted in the Ruff configuration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@misc_scripts/thin_metadata_size_viz.py` around lines 119 - 163, The main() function in thin_metadata_size_viz.py exceeds Ruff's PLR0914 local-variable limit; refactor to reduce locals by extracting validation and parsing into helpers and grouping related values: move argument parsing and conversion into a function (e.g., parse_and_validate_args() that calls gen_parser()), extract the pool/num-thins step validation into validate_steps(min_pool_size, max_pool_size, pool_size_intervals) and validate_steps(min_num_thins, max_num_thins, num_thins_intervals), and return grouped results (e.g., a simple namespace or tuple) that main() then passes to build_arrays(block_size, values) and plot_figure; update main() to call these helpers so it only holds a few variables (block_size, values, and figure).
54-95:⚠️ Potential issue | 🟠 Major
build_arraysexceeds the Ruff local-variable limit and needs the suppression restored or refactoring.The function contains 15 local variables, exceeding the PLR0914 threshold. Either restore the
# noqa: PLR0914suppression or refactor to reduce the number of local variables.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@misc_scripts/thin_metadata_size_viz.py` around lines 54 - 95, The build_arrays function currently exceeds the Ruff local-variable limit (PLR0914); either restore the prior suppression or reduce locals: add a trailing "# noqa: PLR0914" to the def build_arrays signature to silence the rule, or refactor inside build_arrays (collapse unnecessary tuple assignments like "(x_values, y_values, z_values) = ([], [], [])" to single assignments, reuse variables such as reading stdout/stderr inline rather than separate names, and avoid temporary names like x_row/y_row/z_row by appending directly or reusing one row variable) so the local variable count falls below the threshold.
🧹 Nitpick comments (1)
dependency_management/_utils.py (1)
59-59: Prefer a named constant instead of suppressingPLR2004.Line 59 is functionally correct, but extracting
34into a constant keeps intent clearer and avoids inline lint suppression.♻️ Proposed refactor
+MIN_SUPPORTED_FEDORA_RELEASE = 34 ... - if release_number < 34: # noqa PLR2004 + if release_number < MIN_SUPPORTED_FEDORA_RELEASE: raise RuntimeError( - f"release number must be at least 34, was {release_number}" + f"release number must be at least {MIN_SUPPORTED_FEDORA_RELEASE}, was {release_number}" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dependency_management/_utils.py` at line 59, Replace the magic number 34 with a named module-level constant (e.g., MIN_SUPPORTED_RELEASE or RELEASE_NUMBER_THRESHOLD) and use that constant in the conditional inside the function that checks release_number (the line currently reading "if release_number < 34:"); add the constant near the top of dependency_management/_utils.py with a brief descriptive name and remove the inline "noqa PLR2004" comment so the intent is explicit and linting no longer needs suppression.
🤖 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 12-16: The fmt-travis Makefile target currently runs "ruff format"
which mutates files and won't fail CI on formatting drift; change the command in
the fmt-travis recipe to run "ruff format --check" instead so Ruff runs in
check-only mode (leave shfmt -d . as-is); update the fmt-travis target
invocation that references ruff format to ruff format --check to ensure CI
detects formatting issues.
In `@pyproject.toml`:
- Around line 5-10: The Ruff config currently sets select = ["PL"] which omits
the isort rule, so import-sorting checks under [tool.ruff.lint.isort] won't run;
update the Ruff selection to include the "I" rule (e.g., add "I" to the select
array) so isort is enabled and your [tool.ruff.lint.isort] settings
(known-first-party, split-on-trailing-comma) take effect. Target the select
setting under [tool.ruff.lint] in pyproject.toml and add "I" to the list.
In `@release_management/_utils.py`:
- Around line 189-192: The git tag command is currently passing the message with
embedded quotes (f'--message="{message}"') which makes Git store literal quotes;
update the subprocess.run call in the block guarded by verify_tag(tag) to pass
the message as a separate argv item (e.g., use "-m", message or "--message",
message) so the message is passed raw to Git; modify the list passed to
subprocess.run in the same function/block to replace f'--message="{message}"'
with "-m", message.
In `@release_management/create_artifacts.py`:
- Around line 199-204: The broad except in the if __name__ == "__main__": block
around main() triggers PL0718; either narrow the catch to the specific
exceptions main() can raise (replace "except Exception as err:" with specific
exception types returned/raised by main()) or explicitly suppress the lint
warning by adding a Pylint/Ruff suppression on that line (e.g., append "# noqa:
PL0718" or the equivalent Pylint inline disable) while keeping the existing
print(err) and sys.exit(1) behavior; update the except line that references
main() to implement one of these fixes.
In `@release_management/create_release.py`:
- Around line 236-240: The `# noqa PLW0108` suppression is malformed; update the
lambda call inside dry_run_caller to use the correct Ruff syntax by changing the
comment to `# noqa: PLW0108` next to the lambda expression (the call involving
dry_run_caller and RustCrates._publish) so the PLW0108 warning is properly
suppressed.
---
Outside diff comments:
In `@misc_scripts/thin_metadata_size_viz.py`:
- Around line 119-163: The main() function in thin_metadata_size_viz.py exceeds
Ruff's PLR0914 local-variable limit; refactor to reduce locals by extracting
validation and parsing into helpers and grouping related values: move argument
parsing and conversion into a function (e.g., parse_and_validate_args() that
calls gen_parser()), extract the pool/num-thins step validation into
validate_steps(min_pool_size, max_pool_size, pool_size_intervals) and
validate_steps(min_num_thins, max_num_thins, num_thins_intervals), and return
grouped results (e.g., a simple namespace or tuple) that main() then passes to
build_arrays(block_size, values) and plot_figure; update main() to call these
helpers so it only holds a few variables (block_size, values, and figure).
- Around line 54-95: The build_arrays function currently exceeds the Ruff
local-variable limit (PLR0914); either restore the prior suppression or reduce
locals: add a trailing "# noqa: PLR0914" to the def build_arrays signature to
silence the rule, or refactor inside build_arrays (collapse unnecessary tuple
assignments like "(x_values, y_values, z_values) = ([], [], [])" to single
assignments, reuse variables such as reading stdout/stderr inline rather than
separate names, and avoid temporary names like x_row/y_row/z_row by appending
directly or reusing one row variable) so the local variable count falls below
the threshold.
---
Nitpick comments:
In `@dependency_management/_utils.py`:
- Line 59: Replace the magic number 34 with a named module-level constant (e.g.,
MIN_SUPPORTED_RELEASE or RELEASE_NUMBER_THRESHOLD) and use that constant in the
conditional inside the function that checks release_number (the line currently
reading "if release_number < 34:"); add the constant near the top of
dependency_management/_utils.py with a brief descriptive name and remove the
inline "noqa PLR2004" comment so the intent is explicit and linting no longer
needs suppression.
🪄 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: 353a78ab-1e8a-4ab6-b37b-72e4d10a3c9e
📒 Files selected for processing (12)
.github/workflows/main.yml.isort.cfgMakefiledependency_management/_utils.pymisc_scripts/filesystem_size_estimate.pymisc_scripts/filesystem_size_input.pymisc_scripts/thin_metadata_size_viz.pymisc_scripts/update_introspection_data.pypyproject.tomlrelease_management/_utils.pyrelease_management/create_artifacts.pyrelease_management/create_release.py
💤 Files with no reviewable changes (1)
- .isort.cfg
| .PHONY: fmt-travis | ||
| fmt-travis: | ||
| isort --diff --check-only ./blackbox/parse_json.py | ||
| isort --diff --check-only ./dependency_management/* | ||
| isort --diff --check-only ./misc_scripts/ | ||
| isort --diff --check-only ./release_management/* | ||
| black ./blackbox/parse_json.py --check | ||
| black ./dependency_management/ --check | ||
| black ./dependency_management/compare_fedora_versions --check | ||
| black ./misc_scripts/*.py --check | ||
| black ./release_management/ --check | ||
| ruff check --select I | ||
| ruff format | ||
| shfmt -d . |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
ruff format --check flag documentation behavior
💡 Result:
The ruff format --check flag checks if Python files need formatting according to Ruff's rules without modifying them. It exits with exit code 0 if all files are already correctly formatted (no changes needed), 1 if one or more files would be reformatted (indicating formatting issues), and 2 for errors like invalid configuration or internal issues. This is designed for CI/CD pipelines to detect unformatted code without altering files. Without --check, ruff format formats files in-place and exits 0 on success (even if changes were made) or 2 on error. For full options, run ruff format --help.
Citations:
- 1: https://docs.astral.sh/ruff/formatter/
- 2: https://docs.bswen.com/blog/2026-03-29-ruff-cli-flags/
- 3: https://docs.astral.sh/ruff/linter/
- 4: ruff format vs ruff check output and verbose option astral-sh/ruff#10424
- 5: provide a exit code for
ruff format(without --check) for source file syntax error astral-sh/ruff#13419 - 6: Add an
--exit-non-zero-on-fixequivalent to the formatter. astral-sh/ruff#8191
🏁 Script executed:
cat -n MakefileRepository: stratis-storage/ci
Length of output: 587
Use ruff format --check in fmt-travis.
The fmt-travis target is a CI validation step (note shfmt -d . uses check-only mode). However, ruff format rewrites files in-place by default and exits successfully even if changes were made, causing the CI check to pass without detecting formatting drift. Switch to ruff format --check to properly validate formatting in CI without modifying files.
🔧 Proposed fix
fmt-travis:
ruff check --select I
- ruff format
+ ruff format --check
shfmt -d .📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .PHONY: fmt-travis | |
| fmt-travis: | |
| isort --diff --check-only ./blackbox/parse_json.py | |
| isort --diff --check-only ./dependency_management/* | |
| isort --diff --check-only ./misc_scripts/ | |
| isort --diff --check-only ./release_management/* | |
| black ./blackbox/parse_json.py --check | |
| black ./dependency_management/ --check | |
| black ./dependency_management/compare_fedora_versions --check | |
| black ./misc_scripts/*.py --check | |
| black ./release_management/ --check | |
| ruff check --select I | |
| ruff format | |
| shfmt -d . | |
| .PHONY: fmt-travis | |
| fmt-travis: | |
| ruff check --select I | |
| ruff format --check | |
| shfmt -d . |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` around lines 12 - 16, The fmt-travis Makefile target currently runs
"ruff format" which mutates files and won't fail CI on formatting drift; change
the command in the fmt-travis recipe to run "ruff format --check" instead so
Ruff runs in check-only mode (leave shfmt -d . as-is); update the fmt-travis
target invocation that references ruff format to ruff format --check to ensure
CI detects formatting issues.
| [tool.ruff.lint] | ||
| select = ["PL"] | ||
|
|
||
| [tool.ruff.lint.isort] | ||
| known-first-party = ["dbus_python_client_gen"] | ||
| split-on-trailing-comma = false |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Ruff lint select rule "I" isort configuration behavior
💡 Result:
In Ruff, "I" selects the isort ruleset, which includes I001 (unsorted-imports). This rule detects unsorted or unformatted import blocks and can auto-fix them via ruff check --fix. Key behavior: - Enable with [tool.ruff.lint.select] = ["I"] in pyproject.toml or CLI --select I. - Sorting and grouping follow isort-compatible settings under [tool.ruff.lint.isort], like known-first-party, section-order, force-sort-within-sections = true, length-sort = true. - ruff format does NOT sort imports; use ruff check --select I --fix followed by ruff format for full isort + formatting. - I001 fixes de-duplicate, group (stdlib/third-party/first-party/local), and sort imports per config. Some edge cases differ from isort (e.g., length-sort with force-sort-within-sections), but Ruff aims for compatibility and recommends using one tool. Example config: [tool.ruff.lint] select = ["I"] [tool.ruff.lint.isort] # Optional customizations lines-after-imports = 2 Run: ruff check --select I --fix . && ruff format .
Citations:
- 1: Formatter does not sort inputs to match isort profile ["I"] astral-sh/ruff#8612
- 2: Import-sorting (a
ruffapproximation ofisort) astral-sh/ruff#465 - 3: https://stackoverflow.com/questions/78156811/how-do-i-isort-using-ruff
- 4: https://beta.ruff.rs/docs/rules/unsorted-imports/
- 5: https://docs.astral.sh/ruff/rules/unsorted-imports/
- 6: Isort sections no longer respected? astral-sh/ruff#11047
🏁 Script executed:
# Check if there are other ruff configurations in the codebase
fd -type f -name "pyproject.toml" -o -name "ruff.toml" | head -20Repository: stratis-storage/ci
Length of output: 233
🏁 Script executed:
# Check ruff version and any documentation in the repo
find . -type f \( -name "*.md" -o -name "*.rst" -o -name "*.txt" \) | xargs grep -l "ruff\|isort" 2>/dev/null | head -10Repository: stratis-storage/ci
Length of output: 44
Add "I" rule to enable import sorting.
With select = ["PL"], the [tool.ruff.lint.isort] configuration is inert. The "I" rule (isort) must be explicitly selected for import sorting checks to run.
Suggested fix
[tool.ruff.lint]
-select = ["PL"]
+select = ["PL", "I"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [tool.ruff.lint] | |
| select = ["PL"] | |
| [tool.ruff.lint.isort] | |
| known-first-party = ["dbus_python_client_gen"] | |
| split-on-trailing-comma = false | |
| [tool.ruff.lint] | |
| select = ["PL", "I"] | |
| [tool.ruff.lint.isort] | |
| known-first-party = ["dbus_python_client_gen"] | |
| split-on-trailing-comma = false |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pyproject.toml` around lines 5 - 10, The Ruff config currently sets select =
["PL"] which omits the isort rule, so import-sorting checks under
[tool.ruff.lint.isort] won't run; update the Ruff selection to include the "I"
rule (e.g., add "I" to the select array) so isort is enabled and your
[tool.ruff.lint.isort] settings (known-first-party, split-on-trailing-comma)
take effect. Target the select setting under [tool.ruff.lint] in pyproject.toml
and add "I" to the list.
There was a problem hiding this comment.
@coderabbitai I only want to use "I" in formatting step, because that is the way we have always viewed it. I don't think it's semantic as all that.
There was a problem hiding this comment.
@mulkieran, understood — that's a perfectly valid approach. Since the Makefile's fmt/fmt-travis targets invoke ruff check --select I explicitly on the command line, the [tool.ruff.lint.isort] settings in pyproject.toml (e.g. known-first-party, split-on-trailing-comma) will still be picked up at that point. Keeping "I" out of lint's select = ["PL"] cleanly separates formatting concerns from semantic linting concerns. My comment was overstated — no change needed here.
✏️ Learnings added
Learnt from: mulkieran
Repo: stratis-storage/ci PR: 577
File: pyproject.toml:5-10
Timestamp: 2026-04-29T13:54:55.501Z
Learning: In the stratis-storage/ci repository, import sorting (ruff "I" / isort rule) is intentionally treated as a formatting concern, not a linting concern. It is only invoked via `ruff check --select I` in the Makefile `fmt`/`fmt-travis` targets, and is deliberately excluded from `[tool.ruff.lint] select` in `pyproject.toml` (which only contains `["PL"]`). The `[tool.ruff.lint.isort]` settings in `pyproject.toml` are still effective because they are picked up whenever ruff processes isort rules via the CLI flag.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| if not verify_tag(tag): | ||
| subprocess.run( | ||
| ["git", "tag", "--annotate", tag, f'--message="{message}"'], | ||
| check=True, | ||
| ["git", "tag", "--annotate", tag, f'--message="{message}"'], check=True | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and read the actual file to confirm the code
fd -t f "_utils.py" release_management/Repository: stratis-storage/ci
Length of output: 91
🏁 Script executed:
# Read the file to see the actual code at lines 189-192
sed -n '180,200p' release_management/_utils.pyRepository: stratis-storage/ci
Length of output: 527
🏁 Script executed:
# Let's check if there are other usages of subprocess.run with git in the codebase
rg "subprocess\.run.*git" -A 2 -B 1Repository: stratis-storage/ci
Length of output: 632
🏁 Script executed:
# Let's understand how git tag handles the --message argument with quotes
# Create a test to see the actual behavior
cat > /tmp/test_git_args.py << 'EOF'
import subprocess
import tempfile
import os
# Create a temporary git repo for testing
with tempfile.TemporaryDirectory() as tmpdir:
os.chdir(tmpdir)
# Initialize a test repo
subprocess.run(["git", "init"], check=True, capture_output=True)
subprocess.run(["git", "config", "user.email", "test@test.com"], check=True, capture_output=True)
subprocess.run(["git", "config", "user.name", "Test User"], check=True, capture_output=True)
# Create a test commit
with open("test.txt", "w") as f:
f.write("test")
subprocess.run(["git", "add", "test.txt"], check=True, capture_output=True)
subprocess.run(["git", "commit", "-m", "test"], check=True, capture_output=True)
# Test 1: Using the problematic format from the review
message = "Test message"
try:
result = subprocess.run(
["git", "tag", "--annotate", "test-tag-1", f'--message="{message}"'],
check=True, capture_output=True, text=True
)
print("Test 1 (quoted format) succeeded")
except subprocess.CalledProcessError as e:
print(f"Test 1 (quoted format) failed: {e.stderr}")
# Check what was stored
try:
result = subprocess.run(
["git", "tag", "-l", "test-tag-1", "-n"],
check=True, capture_output=True, text=True
)
print(f"Tag content (Test 1): {repr(result.stdout)}")
except:
pass
# Test 2: Using the suggested format
try:
result = subprocess.run(
["git", "tag", "--annotate", "test-tag-2", "-m", message],
check=True, capture_output=True, text=True
)
print("Test 2 (separate argument) succeeded")
except subprocess.CalledProcessError as e:
print(f"Test 2 (separate argument) failed: {e.stderr}")
# Check what was stored
try:
result = subprocess.run(
["git", "tag", "-l", "test-tag-2", "-n"],
check=True, capture_output=True, text=True
)
print(f"Tag content (Test 2): {repr(result.stdout)}")
except:
pass
EOF
python /tmp/test_git_args.pyRepository: stratis-storage/ci
Length of output: 244
🏁 Script executed:
# Let's also verify how git interprets the argument format by checking git documentation
# and test with different message content to see edge cases
cat > /tmp/test_git_args_edge.py << 'EOF'
import subprocess
import tempfile
import os
# Create a temporary git repo for testing
with tempfile.TemporaryDirectory() as tmpdir:
os.chdir(tmpdir)
# Initialize a test repo
subprocess.run(["git", "init"], check=True, capture_output=True)
subprocess.run(["git", "config", "user.email", "test@test.com"], check=True, capture_output=True)
subprocess.run(["git", "config", "user.name", "Test User"], check=True, capture_output=True)
# Create a test commit
with open("test.txt", "w") as f:
f.write("test")
subprocess.run(["git", "add", "test.txt"], check=True, capture_output=True)
subprocess.run(["git", "commit", "-m", "test"], check=True, capture_output=True)
# Test with special characters
test_cases = [
("Message with spaces", "quoted_format"),
("Message with 'single quotes'", "quoted_format_2"),
("Message with special chars: !@#$%", "quoted_format_3"),
]
for message, tag_name in test_cases:
try:
# Using the problematic format
result = subprocess.run(
["git", "tag", "--annotate", tag_name, f'--message="{message}"'],
check=True, capture_output=True, text=True
)
# Retrieve the tag message
result = subprocess.run(
["git", "tag", "-l", tag_name, "-n"],
check=True, capture_output=True, text=True
)
stored_msg = result.stdout.strip()
print(f"Message: {repr(message)}")
print(f"Stored: {repr(stored_msg)}")
print(f"Match: {message in stored_msg}")
print()
except Exception as e:
print(f"Error with message {repr(message)}: {e}")
print()
EOF
python /tmp/test_git_args_edge.pyRepository: stratis-storage/ci
Length of output: 395
Pass the git tag message as a separate argv item.
The current code stores literal quotes in the tag message. With f'--message="{message}"', Git receives the quotes as part of the argument value and stores them literally. Use -m with message as a separate argv item instead.
Suggested fix
- subprocess.run(
- ["git", "tag", "--annotate", tag, f'--message="{message}"'], check=True
- )
+ subprocess.run(["git", "tag", "--annotate", tag, "-m", message], check=True)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if not verify_tag(tag): | |
| subprocess.run( | |
| ["git", "tag", "--annotate", tag, f'--message="{message}"'], | |
| check=True, | |
| ["git", "tag", "--annotate", tag, f'--message="{message}"'], check=True | |
| ) | |
| if not verify_tag(tag): | |
| subprocess.run(["git", "tag", "--annotate", tag, "-m", message], check=True) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@release_management/_utils.py` around lines 189 - 192, The git tag command is
currently passing the message with embedded quotes (f'--message="{message}"')
which makes Git store literal quotes; update the subprocess.run call in the
block guarded by verify_tag(tag) to pass the message as a separate argv item
(e.g., use "-m", message or "--message", message) so the message is passed raw
to Git; modify the list passed to subprocess.run in the same function/block to
replace f'--message="{message}"' with "-m", message.
There was a problem hiding this comment.
@coderabbitai What's wrong with f"--message={message}", though?
There was a problem hiding this comment.
@mulkieran Nothing is wrong with f"--message={message}". The problem in the original code was specifically the extra literal quote characters embedded in the f-string: f'--message="{message}"'. Because Python's subprocess passes each list element as a raw argv item (no shell involved), git receives the string --message="Release v1.0" (with the double-quotes as literal characters) and stores them verbatim in the tag annotation — so the tag message would read "Release v1.0" instead of Release v1.0.
f"--message={message}" (no embedded quotes around the value) is perfectly fine — git's --option=value long-option syntax is standard, and everything after the first = is taken as the value. That form is functionally equivalent to ["--message", message] or ["-m", message].
🧠 Learnings used
Learnt from: mulkieran
Repo: stratis-storage/ci PR: 577
File: pyproject.toml:5-10
Timestamp: 2026-04-29T13:54:55.501Z
Learning: In the stratis-storage/ci repository, import sorting (ruff "I" / isort rule) is intentionally treated as a formatting concern, not a linting concern. It is only invoked via `ruff check --select I` in the Makefile `fmt`/`fmt-travis` targets, and is deliberately excluded from `[tool.ruff.lint] select` in `pyproject.toml` (which only contains `["PL"]`). The `[tool.ruff.lint.isort]` settings in `pyproject.toml` are still effective because they are picked up whenever ruff processes isort rules via the CLI flag.
| if __name__ == "__main__": | ||
| try: | ||
| main() | ||
| except Exception as err: # pylint: disable=broad-except | ||
| except Exception as err: | ||
| print(err) | ||
| sys.exit(1) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n release_management/create_artifacts.py | sed -n '195,210p'Repository: stratis-storage/ci
Length of output: 297
🏁 Script executed:
find . -name "pyproject.toml" -o -name "ruff.toml" -o -name ".ruff.toml" | head -5Repository: stratis-storage/ci
Length of output: 79
🏁 Script executed:
if [ -f "pyproject.toml" ]; then
echo "=== pyproject.toml ===" && cat pyproject.toml
elif [ -f "ruff.toml" ]; then
echo "=== ruff.toml ===" && cat ruff.toml
elif [ -f ".ruff.toml" ]; then
echo "=== .ruff.toml ===" && cat .ruff.toml
fiRepository: stratis-storage/ci
Length of output: 329
🏁 Script executed:
rg -i "too-broad-exception|PL0718|broad" . --type md --type txt 2>/dev/null | head -20Repository: stratis-storage/ci
Length of output: 44
🏁 Script executed:
# Check if there are any existing noqa comments in the file for reference
rg "noqa|pylint|ignore" release_management/create_artifacts.pyRepository: stratis-storage/ci
Length of output: 44
🏁 Script executed:
# Check git history to see if a suppression was removed
git log -p --all -S "noqa" -- release_management/create_artifacts.py 2>/dev/null | head -100Repository: stratis-storage/ci
Length of output: 44
Add a lint suppression comment or narrow this exception handler.
The Ruff configuration enables all Pylint rules (select = ["PL"]), which includes the broad-exception rule (PL0718). The except Exception as err handler at lines 202–204 will trigger a lint failure without a suppression comment or by narrowing the exception to a more specific type.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@release_management/create_artifacts.py` around lines 199 - 204, The broad
except in the if __name__ == "__main__": block around main() triggers PL0718;
either narrow the catch to the specific exceptions main() can raise (replace
"except Exception as err:" with specific exception types returned/raised by
main()) or explicitly suppress the lint warning by adding a Pylint/Ruff
suppression on that line (e.g., append "# noqa: PL0718" or the equivalent Pylint
inline disable) while keeping the existing print(err) and sys.exit(1) behavior;
update the except line that references main() to implement one of these fixes.
There was a problem hiding this comment.
@coderabbitai ruff reviews this file and reports no errors, so I guess it's not bothered.
There was a problem hiding this comment.
@mulkieran, you're right — I apologize for the incorrect comment. I cited rule PL0718 which is not a valid ruff rule code (the corresponding pylint rule W0718 maps to PLW0718 in ruff), and in any case ruff's ["PL"] select doesn't flag except Exception in this file. No change is needed here.
✏️ Learnings added
Learnt from: mulkieran
Repo: stratis-storage/ci PR: 577
File: release_management/create_artifacts.py:199-204
Timestamp: 2026-04-29T13:58:22.708Z
Learning: In the stratis-storage/ci repository, ruff with `select = ["PL"]` in `pyproject.toml` does NOT flag `except Exception as err` in `__main__` guards (e.g., in `release_management/create_artifacts.py`). The broad-exception-caught rule (pylint W0718, ruff PLW0718) is not triggered by the configured rule set, so no suppression comment is needed on such handlers.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: mulkieran
Repo: stratis-storage/ci PR: 577
File: pyproject.toml:5-10
Timestamp: 2026-04-29T13:54:55.501Z
Learning: In the stratis-storage/ci repository, import sorting (ruff "I" / isort rule) is intentionally treated as a formatting concern, not a linting concern. It is only invoked via `ruff check --select I` in the Makefile `fmt`/`fmt-travis` targets, and is deliberately excluded from `[tool.ruff.lint] select` in `pyproject.toml` (which only contains `["PL"]`). The `[tool.ruff.lint.isort]` settings in `pyproject.toml` are still effective because they are picked up whenever ruff processes isort rules via the CLI flag.
| dry_run_caller( | ||
| "__main__.RustCrates._publish", | ||
| lambda: RustCrates._publish(), | ||
| lambda: RustCrates._publish(), # noqa PLW0108 | ||
| skip=namespace.no_publish, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the file and surrounding context
cat -n release_management/create_release.py | sed -n '230,245p'Repository: stratis-storage/ci
Length of output: 677
🏁 Script executed:
# Look for Ruff configuration in the repo
find . -type f \( -name "pyproject.toml" -o -name ".ruff.toml" -o -name "ruff.toml" \) | head -5Repository: stratis-storage/ci
Length of output: 79
🏁 Script executed:
# Search for existing noqa patterns in codebase to see what style is used
rg "noqa" --type py | head -20Repository: stratis-storage/ci
Length of output: 242
🏁 Script executed:
# Check Ruff configuration in pyproject.toml
cat pyproject.toml | grep -A 30 "\[tool.ruff"Repository: stratis-storage/ci
Length of output: 306
🌐 Web query:
Ruff noqa syntax space vs colon PLW0108
💡 Result:
In Ruff, the correct noqa syntax for suppressing specific rules like PLW0108 (unnecessary-lambda) uses a space after the # and a colon after "noqa", with optional space after "noqa:" before the code: # noqa: PLW0108 or # noqa:PLW0108. Ruff's official documentation specifies: "add # noqa: {code} to the end of the line". The full inline suppression parsing allows optional whitespace after the # symbol and after "noqa", but requires a colon ":" after "noqa", followed by rule codes (uppercase letters + digits, separated by whitespace or commas). Historically, #noqa: N805 (no space after #) was ignored, but Ruff now accepts noqa directives even without the preceding space after # (fixed in PR #5532 around July 2023). There is no dedicated rule enforcing or flagging missing spaces or colons in noqa comments (unlike flake8-noqa's NQA001-NQA004). Discussions exist about enforcing a space after the colon (e.g., # noqa:PLW0108 vs # noqa: PLW0108), but Ruff parses both flexibly without strict enforcement. For PLW0108 specifically, examples in docs and issues use the standard # noqa: PLW0108 format. To suppress on a line triggering PLW0108: df.apply(lambda x: str(x)) # noqa: PLW0108 Both space and no-space after colon work due to tolerant parsing ("optional whitespace after noqa, and followed by the symbol :"), but the documented canonical form includes the space after colon.
Citations:
- 1: https://docs.astral.sh/ruff/linter/
- 2: Consider either reporting or accepting #noqa (without space) astral-sh/ruff#5177
- 3: Request: Enforce space after colon (
:) for special comments astral-sh/ruff#10160 - 4: Support
flake8-noqaastral-sh/ruff#3211
🏁 Script executed:
# Double-check if this is the only instance causing issues in that section
rg "noqa PLW0108|noqa: PLW0108" release_management/create_release.pyRepository: stratis-storage/ci
Length of output: 121
🏁 Script executed:
# Verify the complete context around that line to ensure accuracy
sed -n '234,241p' release_management/create_release.pyRepository: stratis-storage/ci
Length of output: 385
Add colon to the noqa directive for correct Ruff syntax.
The # noqa PLW0108 comment is missing the required colon. Ruff expects # noqa: PLW0108 to properly suppress the PLW0108 (unnecessary-lambda) warning.
Suggested fix
- lambda: RustCrates._publish(), # noqa PLW0108
+ lambda: RustCrates._publish(), # noqa: PLW0108📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| dry_run_caller( | |
| "__main__.RustCrates._publish", | |
| lambda: RustCrates._publish(), | |
| lambda: RustCrates._publish(), # noqa PLW0108 | |
| skip=namespace.no_publish, | |
| ) | |
| dry_run_caller( | |
| "__main__.RustCrates._publish", | |
| lambda: RustCrates._publish(), # noqa: PLW0108 | |
| skip=namespace.no_publish, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@release_management/create_release.py` around lines 236 - 240, The `# noqa
PLW0108` suppression is malformed; update the lambda call inside dry_run_caller
to use the correct Ruff syntax by changing the comment to `# noqa: PLW0108` next
to the lambda expression (the call involving dry_run_caller and
RustCrates._publish) so the PLW0108 warning is properly suppressed.
mulkieran
force-pushed
the
replace-linters-and-formatters-with-ruff
branch
3 times, most recently
from
bc11392 to
8424517
Compare
Signed-off-by: mulhern <amulhern@redhat.com>
mulkieran
force-pushed
the
replace-linters-and-formatters-with-ruff
branch
from
8424517 to
79c04a6
Compare
2 participants
Related stratis-storage/project#64
Summary by CodeRabbit
Release Notes
pyproject.tomlfor simplified project setup and maintenance