🧹 Remove unused argument from_version in _migrate_schema#14
Conversation
The `_migrate_schema` method in `RuleEngine` acts as a placeholder and simply returned the data without using the `from_version` argument. This commit removes the unused argument to improve code maintainability and cleanliness. * Removed `from_version` parameter in `_migrate_schema` method in `rules.py`. * Updated the caller in `load_rules` method in `rules.py`. * Updated the `test_migrate_schema_called_for_same_supported_version` test in `test_rules.py`. * Fixed a linting issue where Ruff formatting was aggressively converting `TypeAlias` to `type` in the `sample_type_aliases.py` test fixture, breaking backward compatibility tests. Added this file to `ruff.toml` exclusions. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Pull request overview
This PR cleans up the RuleEngine schema migration placeholder by removing an unused argument, and adjusts linting/configuration so a parsing fixture preserves both legacy and 3.12+ type-alias syntaxes.
Changes:
- Removed unused
from_versionparameter fromRuleEngine._migrate_schemaand updated call sites/tests. - Restored
tests/fixtures/sample_type_aliases.pyto useTypeAliasassignments for pre-3.12 aliases. - Excluded the type-alias fixture from Ruff discovery via
ruff.tomlexclude list.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/exportify/export_manager/rules.py |
Removes unused _migrate_schema parameter and updates call site. |
tests/test_rules.py |
Updates test to match _migrate_schema signature change. |
tests/fixtures/sample_type_aliases.py |
Restores legacy TypeAlias-style aliases alongside 3.12 type aliases for parser tests. |
ruff.toml |
Excludes the fixture from Ruff traversal to avoid automated rewrites. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "venv", | ||
| "typings", | ||
| "tests/fixtures/malformed.py", | ||
| "tests/fixtures/sample_type_aliases.py", |
| from typing import TypeAlias | ||
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…378011 Signed-off-by: Adam Poulemanos <89049923+bashandbone@users.noreply.github.com>
🎯 What: The unused argument
from_versionwas removed from the_migrate_schemamethod insrc/exportify/export_manager/rules.pyand its callers were updated. Additionally,tests/fixtures/sample_type_aliases.pywas excluded fromruff.tomlauto-formatting, and the file was restored to properly testTypeAliasvstypeparsing.💡 Why:
_migrate_schemaacts as a placeholder and doesn't use thefrom_versionargument. Removing it improves code maintainability.tests/fixtures/sample_type_aliases.pycontains pre-3.12 syntax intentionally to test the AST parser, so it should not be updated by linting tools to the 3.12+ syntax.✅ Verification: Verified by running
uv run pytest,uv run ruff check src/ tests/anduv run ty checkand making sure that they all pass successfully.✨ Result: Improved codebase maintainability by cleaning up unused arguments and preventing aggressive auto-formatting in test fixtures.
PR created automatically by Jules for task 6609124693223378011 started by @bashandbone