add ruff check for the whole project#504
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request enables ruff linting for the entire project by configuring ruff checks and applying automatic fixes across the codebase.
Changes:
- Added ruff.toml configuration file excluding certain directories and ignoring E741 rule
- Updated GitHub Actions workflow to run
ruff checkon the entire project - Reorganized imports to use explicit module paths (e.g.,
mytoncore.utilsinstead of wildcard imports) - Removed unused variables and imports throughout the codebase
- Improved code style: removed unnecessary f-strings, improved exception handling, fixed comparison operators
- Deleted unused file
mytoncore/tonblocksscanner.py
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| ruff.toml | Added ruff configuration excluding mypylib, mypyconsole, and tests directories |
| .github/workflows/tests.yml | Updated workflow to run ruff check on entire project |
| mytoncore/init.py | Refactored to export only MyTonCore, removing wildcard exports |
| mytoncore/utils.py | Fixed bare except clause to catch Exception explicitly |
| mytoncore/mytoncore.py | Removed unused variables, improved comparison operators, fixed exception handling |
| mytoncore/telemetry.py | Improved truthiness check for pid validation |
| mytoncore/functions.py | Added explicit imports instead of wildcard, removed unused result variables |
| mytoncore/tonblocksscanner.py | Deleted entire unused file (208 lines) |
| mytoninstaller/*.py | Removed unused imports and variables, improved exception handling, fixed f-strings |
| modules/*.py | Added TYPE_CHECKING imports for type hints, removed unused import, fixed string formatting |
| tests/integration/*.py | Updated imports to use mytoncore.utils explicitly |
| tools/inject_backup_node_keys.py | Removed unnecessary f-string formatting |
| scripts/install.py | Fixed multiline if statement formatting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # user = local.buffer.user | ||
| # args = ["chown", "-R", user + ':' + user, localConfigPath] |
There was a problem hiding this comment.
The chown operation was commented out rather than removed. This code was setting ownership of the local config file to the user. Commenting it out may cause permission issues if the file is created as root but needs to be accessible by a non-root user. Consider whether this operation is still needed or if it should be fully removed if truly unnecessary.
| exclude = [ | ||
| "mypylib", | ||
| "mypyconsole", | ||
| "tests", |
There was a problem hiding this comment.
The ruff configuration excludes the 'tests' directory from linting, but changes were made to test files in this PR (e.g., test_btc_teleport_commands.py, test_basic_commands.py, test_backup_commands.py). This creates an inconsistency - if tests are excluded from ruff checks, they should not have been modified as part of this ruff cleanup. Consider either removing 'tests' from the exclude list or reverting the test file changes.
| "tests", |
| from modules.nominator_pool import NominatorPoolModule | ||
| from modules.single_pool import SingleNominatorModule |
There was a problem hiding this comment.
The import from modules.pool import PoolModule was removed, but PoolModule is still used as a base class by NominatorPoolModule and SingleNominatorModule in their respective files (modules/nominator_pool.py and modules/single_pool.py). This removal will cause import errors when those modules are loaded.
| from modules.nominator_pool import NominatorPoolModule | |
| from modules.single_pool import SingleNominatorModule | |
| try: | |
| from modules.nominator_pool import NominatorPoolModule | |
| except ImportError: | |
| NominatorPoolModule = None # type: ignore[assignment] | |
| try: | |
| from modules.single_pool import SingleNominatorModule | |
| except ImportError: | |
| SingleNominatorModule = None # type: ignore[assignment] |
| except Exception as e: | ||
| color_print(f"{{red}}Could not create backup: {e}{{endc}}") |
There was a problem hiding this comment.
The exception handling was improved to catch the exception as e, but the error message now exposes the exception details directly in the formatted string without sanitization. Consider whether this could leak sensitive information in error scenarios, especially since this is related to backup operations which may involve file paths and system information.
| except Exception as e: | |
| color_print(f"{{red}}Could not create backup: {e}{{endc}}") | |
| except Exception: | |
| color_print("{red}Could not create backup{endc}") |
| except Exception: | ||
| pass |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| except Exception: | |
| pass | |
| except Exception as exc: | |
| # Skip lite servers that fail the health check, but log the error for diagnostics. | |
| local.add_log(f"Lite server at index {index} failed health check: {exc}", "debug") |
| except Exception: | ||
| pass |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| except Exception: | |
| pass | |
| except Exception as e: | |
| self.local.add_log(f'Failed to send duplicate file {filePath}: {e}', 'warning') |
| except Exception: | ||
| pass |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| except Exception: | |
| pass | |
| except ValueError: | |
| # Skip wallet names that do not have a numeric suffix | |
| continue |
| except Exception: | ||
| pass |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| except Exception: | |
| pass | |
| except (TypeError, ValueError): | |
| # If conversion fails, leave result as-is (e.g., keep it as a string) | |
| return result |
| try: | ||
| workchain, addr, bounceable = self.ParseAddrB64(inputAddr) | ||
| except: pass | ||
| except Exception: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| except Exception: | |
| except Exception: | |
| # Intentionally ignore parsing errors and return None for invalid/unparsable addresses. |
| try: | ||
| data = json.loads(data) | ||
| except: pass | ||
| except Exception: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| except Exception: | |
| except Exception: | |
| # If data is not valid JSON or is already a non-string object, keep the original value. |
No description provided.