Add pre-commit configuration with ruff, pyright, and other helpful hooks#18
Add pre-commit configuration with ruff, pyright, and other helpful hooks#18dev-ankit wants to merge 2 commits into
Conversation
- Add .pre-commit-config.yaml with comprehensive linting and formatting hooks - Ruff for linting and formatting (replaces flake8, isort, black) - Pyright for type checking (Astral's type checker) - Standard pre-commit hooks for file cleanup and validation - Bandit for security checks - Markdownlint for markdown formatting - Interrogate for docstring coverage - Add root pyproject.toml for monorepo-level tool configuration - Configure ruff with sensible defaults for Python 3.8+ - Configure pyright for basic type checking - Configure bandit to skip test directories - Configure interrogate for docstring coverage reporting - Add GitHub workflow .github/workflows/pre-commit.yml - Runs on workflow_dispatch for manual triggering - Runs on push and pull requests to main branch - Uses prek instead of pre-commit binary - Caches pre-commit hooks for faster runs - Update .gitignore to exclude pre-commit cache and interrogate badge - Apply automatic formatting fixes from ruff and ruff-format to all Python files Note: Using --no-verify for initial commit. Future commits will be validated by hooks.
Documents all issues found by pre-commit hooks: - 91 ruff linting issues across Python files - Multiple pyright type checking issues - 71+ markdownlint formatting issues Includes: - Detailed file-by-file breakdown of issues - Specific line numbers and suggested fixes - Configuration options to relax rules - Recommended fix order and estimated effort - Quick win approach for minimal changes
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive pre-commit hook configuration to the monorepo, including linting, formatting, type checking, and security scanning tools. The changes also apply automatic formatting fixes from ruff and ruff-format across all Python files in the repository.
Changes:
- Adds
.pre-commit-config.yamlwith ruff (linting/formatting), pyright (type checking), bandit (security), markdownlint, interrogate (docstring coverage), and standard pre-commit hooks - Adds root
pyproject.tomlwith monorepo-level configuration for ruff, pyright, bandit, and interrogate - Adds
.github/workflows/pre-commit.ymlworkflow using prek for automated pre-commit checks in CI - Applies automatic formatting fixes to all Python files across tools (import sorting, quote style, line length, etc.)
- Updates
.gitignoreto exclude pre-commit cache and interrogate badge
Reviewed changes
Copilot reviewed 35 out of 36 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
.pre-commit-config.yaml |
Configures pre-commit hooks with ruff, pyright, bandit, markdownlint, interrogate, and standard file checks |
pyproject.toml |
Root configuration for monorepo-level tooling with sensible defaults for Python 3.8+ |
.github/workflows/pre-commit.yml |
CI workflow to run pre-commit checks using prek on push and PR to main |
.gitignore |
Adds ignores for pre-commit cache and interrogate badge |
tools/wt-worktree/**/*.py |
Automatic formatting fixes: import sorting, quote style, line length, spacing |
tools/locust-compare/**/*.py |
Automatic formatting fixes: import sorting, quote style, line length, spacing |
tools/config-utils/**/*.py |
Automatic formatting fixes: import sorting, quote style, line length, spacing |
*.md files |
Markdown formatting fixes (blank lines, trailing whitespace) |
Agents.md |
Minor formatting and whitespace fixes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| f"Unsupported shell: {shell}\n" | ||
| f"Supported shells: bash, zsh, fish" | ||
| ) | ||
| raise ValueError(f"Unsupported shell: {shell}\n" f"Supported shells: bash, zsh, fish") |
There was a problem hiding this comment.
The string concatenation using adjacent f-strings is technically valid but reduces readability. The formatter created two adjacent string literals that will be concatenated at compile time. Consider using a single f-string or parentheses for clarity.
| raise ConfigError( | ||
| "TOML support not available. " | ||
| "Please install tomli: pip install tomli" | ||
| "TOML support not available. " "Please install tomli: pip install tomli" |
There was a problem hiding this comment.
The string concatenation using adjacent f-strings reduces readability. Consider using a single f-string or explicit concatenation with + operator.
| git.run_git(["branch", "-m", "main"], cwd=repo_path) | ||
|
|
||
| yield repo_path | ||
| return repo_path |
There was a problem hiding this comment.
Changing yield to return in this fixture breaks the cleanup logic. The fixture creates a git repository and should use yield to ensure it stays available during the test. With return, the fixture completes immediately and the repository may be cleaned up before tests that depend on it can use it. The original yield pattern was correct for fixtures that need teardown logic.
| return repo_path | |
| yield repo_path |
| git.run_git(["push", "-u", "origin", "main"], cwd=git_repo) | ||
|
|
||
| yield git_repo, remote_path | ||
| return git_repo, remote_path |
There was a problem hiding this comment.
Changing yield to return in this fixture breaks the cleanup and fixture lifecycle. This fixture creates resources (git repository with remote) that need to remain available throughout the test. The cleanup of temp_dir (which git_repo depends on) happens after the yield, so using return means this fixture completes immediately and dependent cleanup may happen out of order. The original yield pattern was correct.
| return git_repo, remote_path | |
| yield git_repo, remote_path |
| return | ||
| EXIT_ERROR, | ||
| ) | ||
| return None |
There was a problem hiding this comment.
The return type of this function is bool (returns True if deleted, False if cancelled), but line 289 now returns None instead of False. This breaks the function contract and could cause issues where the caller expects a boolean value. The original code had return (which implicitly returns None), but after calling error() with an exit code, the function would have exited the program. Since the exit code is provided to error(), this line should not be reached. However, if someone removes the exit behavior from error(), this will break. The safer fix is to keep return False to match the function's return type annotation.
| raise git.GitError( | ||
| f"Path {wt_path} already exists. " | ||
| f"Please remove it or choose a different name." | ||
| f"Path {wt_path} already exists. " f"Please remove it or choose a different name." |
There was a problem hiding this comment.
The string concatenation using adjacent f-strings is technically valid but less readable than a single f-string. This change appears to have been made by the auto-formatter, but it's worth noting that the original multi-line string was more clear. Consider using parentheses with a single f-string or triple-quoted strings for better readability.
Add .pre-commit-config.yaml with comprehensive linting and formatting hooks
Add root pyproject.toml for monorepo-level tool configuration
Add GitHub workflow .github/workflows/pre-commit.yml
Update .gitignore to exclude pre-commit cache and interrogate badge
Apply automatic formatting fixes from ruff and ruff-format to all Python files
Note: Using --no-verify for initial commit. Future commits will be validated by hooks.