Replace linters and formatters with ruff #577
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,13 @@ | ||||||||||||||||||||||||||
| [tool.ruff] | ||||||||||||||||||||||||||
| target-version = "py312" | ||||||||||||||||||||||||||
| line-length = 88 | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| [tool.ruff.lint] | ||||||||||||||||||||||||||
| select = ["PL"] | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| [tool.ruff.lint.isort] | ||||||||||||||||||||||||||
| known-first-party = ["dbus_python_client_gen"] | ||||||||||||||||||||||||||
| split-on-trailing-comma = false | ||||||||||||||||||||||||||
|
Comment on lines
+5
to
+10
There was a problem hiding this comment. 🧩 Analysis chain🌐 Web query:
💡 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:
🏁 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 Suggested fix [tool.ruff.lint]
-select = ["PL"]
+select = ["PL", "I"]📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Member
Author
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.
✏️ Learnings added
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| [tool.ruff.format] | ||||||||||||||||||||||||||
| skip-magic-trailing-comma = true | ||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -101,7 +101,7 @@ def edit_specfile( | |||||||||||||||||
| if release is not None: | ||||||||||||||||||
| spec.release = release | ||||||||||||||||||
| if sources is not None: | ||||||||||||||||||
| with spec.sources() as entries: | ||||||||||||||||||
| for index, value in enumerate(sources): | ||||||||||||||||||
| entries[index].location = value | ||||||||||||||||||
| if arbitrary is not None: | ||||||||||||||||||
|
|
@@ -154,7 +154,7 @@ def get_package_info(manifest_abs_path, package_name) -> tuple[Version, Any]: | |||||||||||||||||
|
|
||||||||||||||||||
| package = metadata["package"] | ||||||||||||||||||
| assert package["name"] == package_name, ( | ||||||||||||||||||
| f"package name in Cargo.toml ({package['name']}) != specified" | ||||||||||||||||||
| f"package name ({package_name})" | ||||||||||||||||||
| ) | ||||||||||||||||||
| github_repo = urlparse(package["repository"].rstrip("/")) | ||||||||||||||||||
|
|
@@ -188,8 +188,7 @@ def set_tag(tag, message): | |||||||||||||||||
| """ | ||||||||||||||||||
| if not verify_tag(tag): | ||||||||||||||||||
| subprocess.run( | ||||||||||||||||||
| ["git", "tag", "--annotate", tag, f"--message={message}"], check=True | ||||||||||||||||||
| ) | ||||||||||||||||||
|
Comment on lines
189
to
192
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 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
Suggested change
🤖 Prompt for AI Agents
Member
Author
There was a problem hiding this comment. @coderabbitai What's wrong with There was a problem hiding this comment.
🧠 Learnings used |
||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -230,10 +229,7 @@ def create_release( | |||||||||||||||||
| repo = git.get_repo(repository.path.strip("/")) | ||||||||||||||||||
|
|
||||||||||||||||||
| release = repo.create_git_release( | ||||||||||||||||||
| tag, f"Version {release_version}", f"See {changelog_url}", draft=True | ||||||||||||||||||
| ) | ||||||||||||||||||
|
|
||||||||||||||||||
| for asset in [] if additional_assets is None else additional_assets: | ||||||||||||||||||
|
|
@@ -339,7 +335,7 @@ def get_changelog_url(repository_url, branch): | |||||||||||||||||
| """ | ||||||||||||||||||
| changelog_url = f"{repository_url}/blob/{branch}/CHANGES.txt" | ||||||||||||||||||
| requests_var = requests.get(changelog_url, timeout=30) | ||||||||||||||||||
| if requests_var.status_code != requests.codes.ok: | ||||||||||||||||||
| raise RuntimeError(f"Page at URL {changelog_url} not found") | ||||||||||||||||||
|
|
||||||||||||||||||
| return changelog_url | ||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -133,11 +133,7 @@ def _stratisd_artifacts(namespace): | |
| ) | ||
| print(os.path.relpath(source_tarfile_path)) | ||
|
|
||
| vendor_tarfile_name = vendor(manifest_abs_path, release_version, filterer=filtered) | ||
|
|
||
| vendor_tarfile_path = os.path.join(output_path, vendor_tarfile_name) | ||
|
|
||
|
|
@@ -171,11 +167,7 @@ def _stratis_cli_artifacts(namespace): | |
| source_version, pre=namespace.pre_release, post=namespace.post_release | ||
| ) | ||
|
|
||
| source_tarfile = make_source_tarball("stratis-cli", release_version, output_path) | ||
|
|
||
| def remove_stratisd_requires(spec): | ||
| """ | ||
|
|
@@ -207,6 +199,6 @@ def remove_stratisd_requires(spec): | |
| if __name__ == "__main__": | ||
| try: | ||
| main() | ||
| except Exception as err: | ||
| print(err) | ||
| sys.exit(1) | ||
|
Comment on lines
199
to
204
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 ( 🤖 Prompt for AI Agents
Member
Author
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.
✏️ Learnings added
🧠 Learnings used |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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:
ruff format(without --check) for source file syntax error astral-sh/ruff#13419--exit-non-zero-on-fixequivalent to the formatter. astral-sh/ruff#8191🏁 Script executed:
Repository: stratis-storage/ci
Length of output: 587
Use
ruff format --checkinfmt-travis.The
fmt-travistarget is a CI validation step (noteshfmt -d .uses check-only mode). However,ruff formatrewrites files in-place by default and exits successfully even if changes were made, causing the CI check to pass without detecting formatting drift. Switch toruff format --checkto properly validate formatting in CI without modifying files.🔧 Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents