|
| 1 | +# Pre-commit Issues TODO |
| 2 | + |
| 3 | +This document tracks all the changes needed to pass pre-commit hooks. |
| 4 | + |
| 5 | +## Summary |
| 6 | + |
| 7 | +- **Ruff**: 91 issues across Python files |
| 8 | +- **Pyright**: Multiple type checking issues |
| 9 | +- **Markdownlint**: 71+ markdown formatting issues |
| 10 | +- **Other hooks**: All passing ✅ |
| 11 | + |
| 12 | +--- |
| 13 | + |
| 14 | +## 1. Ruff Issues (Python Linting & Formatting) |
| 15 | + |
| 16 | +### tools/config-utils/cli.py |
| 17 | + |
| 18 | +**Issues to fix:** |
| 19 | + |
| 20 | +1. Line 34: Remove unused variable `current_depth` |
| 21 | + |
| 22 | + ```python |
| 23 | + # REMOVE: current_depth = len(parent_key.split(sep)) if parent_key else 0 |
| 24 | + ``` |
| 25 | + |
| 26 | +2. Lines 95, 256, 364: Replace `open()` with `Path.open()` |
| 27 | + - Import `from pathlib import Path` |
| 28 | + - Replace `open(file_path)` with `Path(file_path).open()` |
| 29 | + - Replace `open(output_path, 'w')` with `Path(output_path).open('w')` |
| 30 | + |
| 31 | +3. Line 130: Function `perform_set_operation` is too complex |
| 32 | + - PLR0912: Too many branches (24 > 12) |
| 33 | + - PLR0915: Too many statements (53 > 50) |
| 34 | + - **Action**: Consider refactoring into smaller functions |
| 35 | + |
| 36 | +4. Lines 183, 184: Use set comprehensions instead of generators |
| 37 | + |
| 38 | + ```python |
| 39 | + # CHANGE FROM: |
| 40 | + items1 = set((k, make_hashable(v)) for k, v in flat1.items()) |
| 41 | + # TO: |
| 42 | + items1 = {(k, make_hashable(v)) for k, v in flat1.items()} |
| 43 | + ``` |
| 44 | + |
| 45 | +5. Lines 228, 306, 394: Line too long (>100 chars) |
| 46 | + - Line 228: Docstring for `main()` - split across lines |
| 47 | + - Line 306: Error message - split string concatenation |
| 48 | + - Line 394: Help text - split across lines |
| 49 | + |
| 50 | +6. Lines 246, 294: Unused function argument `format` |
| 51 | + - Either use the parameter or prefix with underscore: `_format` |
| 52 | + |
| 53 | +### tools/config-utils/tests/test_set_operations.py |
| 54 | + |
| 55 | +**Issues to fix:** |
| 56 | + |
| 57 | +1. Lines 52, 65, 101, 107, 147, 159, 184, 191, 229, 236, 272, 279, 315, 322, 348, 354: Replace `open()` with `Path.open()` |
| 58 | + - Import `from pathlib import Path` |
| 59 | + - Replace all instances of `open(temp_file, 'w')` pattern |
| 60 | + |
| 61 | +### tools/locust-compare/compare_runs.py |
| 62 | + |
| 63 | +**Issues to fix:** |
| 64 | + |
| 65 | +1. Lines 51, 231, 248, 272, 329, 378, 409, 426, 434: Replace `open()` with `Path.open()` |
| 66 | + - Import `from pathlib import Path` |
| 67 | + - Replace `open()` calls with `Path().open()` |
| 68 | + |
| 69 | +2. Line 79: Function `extract_from_html` is too complex |
| 70 | + - PLR0912: Too many branches (15 > 12) |
| 71 | + - **Action**: Consider extracting parsing logic into helper functions |
| 72 | + |
| 73 | +3. Line 166: Function `load_report` is too complex |
| 74 | + - PLR0912: Too many branches (18 > 12) |
| 75 | + - **Action**: Split file type handling into separate functions |
| 76 | + |
| 77 | +4. Lines 208, 214, 221: Replace `os.path` with `pathlib` |
| 78 | + - PTH118: `os.path.join()` → use `/` operator with Path |
| 79 | + - PTH119: `os.path.basename()` → use `Path.name` |
| 80 | + - PTH123: `os.path.dirname()` → use `Path.parent` |
| 81 | + |
| 82 | +5. Line 319: Ambiguous variable name `l` (E741) |
| 83 | + - Rename to something descriptive like `line` or `item` |
| 84 | + |
| 85 | +6. Line 521: Line too long |
| 86 | + - Split the line |
| 87 | + |
| 88 | +### tools/locust-compare/tests/*.py |
| 89 | + |
| 90 | +**Multiple test files with PTH123 violations:** |
| 91 | + |
| 92 | +- `conftest.py`: Lines 14, 27, 28, 29, 30, 52, 56, 60, 64, 75, 79, 91, 93 |
| 93 | +- `test_compare_reports.py`: Lines 6, 14 |
| 94 | +- `test_html_extraction.py`: Lines 11, 24, 37, 46, 62, 78, 92, 109, 124 |
| 95 | +- `test_html_feature_map.py`: Line 6 |
| 96 | +- `test_load_report.py`: Lines 11, 27, 43, 59, 75, 96, 115, 144, 171 |
| 97 | +- `test_markdown_output.py`: Lines 11, 22 |
| 98 | +- `test_metrics.py`: Line 6 |
| 99 | +- `test_row.py`: Line 6 |
| 100 | +- `test_utils.py`: Lines 7, 17, 29, 39, 45, 56, 68, 80, 92, 104, 116, 123, 132 |
| 101 | +- `test_zip_support.py`: Lines 17, 36, 61, 86, 113, 138, 165, 191, 217, 241, 262 |
| 102 | + |
| 103 | +**Action**: Replace all `open()` with `Path().open()` in test files |
| 104 | + |
| 105 | +### tools/wt-worktree/wt/*.py |
| 106 | + |
| 107 | +**Issues to fix:** |
| 108 | + |
| 109 | +- `cli.py`: Lines 15, 22 - PTH123 violations |
| 110 | +- `config.py`: Lines 14, 102, 127, 205 - PTH123 violations; Line 14 - PTH118 violation |
| 111 | +- `git.py`: Lines 47, 73, 91, 94 - PTH119 violations |
| 112 | +- `prompts.py`: Line 86 - PTH123 violation |
| 113 | +- `shell.py`: Lines 129, 130 - PTH118 violations |
| 114 | +- `worktree.py`: Lines 72, 84, 89, 103, 107, 121, 151, 158, 176 - PTH119 violations; Line 166 - PTH118 violation |
| 115 | + |
| 116 | +**Action**: Replace `os.path` functions with `pathlib.Path` equivalents |
| 117 | + |
| 118 | +### tools/wt-worktree/tests/*.py |
| 119 | + |
| 120 | +**Issues to fix:** |
| 121 | + |
| 122 | +- `conftest.py`: Line 31 - PTH123 violation |
| 123 | +- `test_cli.py`: Lines 133, 193, 252, 300, 327, 349, 414, 506, 652 - PTH123 violations |
| 124 | +- `test_config.py`: Lines 12, 32, 51, 95, 132, 182, 202, 244, 284, 330 - PTH123 violations |
| 125 | +- `test_git.py`: Lines 17, 25, 50, 61, 87 - PTH123 violations |
| 126 | +- `test_worktree.py`: Lines 149, 165, 186, 236, 289, 363, 418, 569, 673 - PTH123 violations |
| 127 | + |
| 128 | +**Action**: Replace all `open()` with `Path().open()` |
| 129 | + |
| 130 | +--- |
| 131 | + |
| 132 | +## 2. Pyright Issues (Type Checking) |
| 133 | + |
| 134 | +### General approach |
| 135 | + |
| 136 | +1. Add missing type hints to function signatures |
| 137 | +2. Import `from typing import` necessary types (Dict, List, Optional, Any, etc.) |
| 138 | +3. Fix type mismatches |
| 139 | +4. Handle Optional/None cases properly |
| 140 | + |
| 141 | +**Files with type issues:** |
| 142 | + |
| 143 | +- All Python files in `tools/config-utils/`, `tools/locust-compare/`, and `tools/wt-worktree/` |
| 144 | + |
| 145 | +**Recommended approach:** |
| 146 | + |
| 147 | +- Run `prek run pyright` after fixing ruff issues |
| 148 | +- Fix type errors incrementally, file by file |
| 149 | +- Start with main modules before test files |
| 150 | + |
| 151 | +--- |
| 152 | + |
| 153 | +## 3. Markdownlint Issues |
| 154 | + |
| 155 | +### README.md (root) |
| 156 | + |
| 157 | +- Line 12: Line too long (83 > 80) |
| 158 | +- **Action**: Split long lines or add `.markdownlint.json` to relax line length |
| 159 | + |
| 160 | +### Agents.md |
| 161 | + |
| 162 | +- Lines 70, 74, 82: Line too long |
| 163 | +- **Action**: Reformat or configure longer line length for docs |
| 164 | + |
| 165 | +### tools/config-utils/README.md |
| 166 | + |
| 167 | +**Issues:** |
| 168 | + |
| 169 | +- MD024: Multiple duplicate headings ("Options", "Examples") |
| 170 | +- MD013: Multiple line length violations |
| 171 | +- MD040: Missing language specifiers on fenced code blocks |
| 172 | + |
| 173 | +**Actions:** |
| 174 | + |
| 175 | +1. Make headings unique (e.g., "Options" → "Command Options", "Subcommand Options") |
| 176 | +2. Split long lines or relax line length |
| 177 | +3. Add language to code blocks: ` ```bash ` or ` ```yaml ` |
| 178 | + |
| 179 | +### tools/locust-compare/README.md |
| 180 | + |
| 181 | +**Issues:** |
| 182 | + |
| 183 | +- MD013: Line length violations (lines 3, 10, 23, 107, 154, 166, 167, 172, 189) |
| 184 | +- MD040: Missing code block languages (lines 76, 140, 151, 177) |
| 185 | +- MD033: Inline HTML on line 107 |
| 186 | + |
| 187 | +**Actions:** |
| 188 | + |
| 189 | +1. Split long lines |
| 190 | +2. Add ` ```bash ` or ` ```python ` to code blocks |
| 191 | +3. Replace inline HTML `<img>` with markdown image syntax |
| 192 | + |
| 193 | +### tools/wt-worktree/README.md |
| 194 | + |
| 195 | +**Issues:** |
| 196 | + |
| 197 | +- MD013: Line length violations (lines 3, 7, 73, 75, 140) |
| 198 | +- MD040: Missing code block languages (lines 165, 224) |
| 199 | + |
| 200 | +**Actions:** |
| 201 | + |
| 202 | +1. Split long lines |
| 203 | +2. Add ` ```bash ` to code blocks |
| 204 | + |
| 205 | +### tools/wt-worktree/PRD.md |
| 206 | + |
| 207 | +**Issues:** |
| 208 | + |
| 209 | +- MD013: Line length violations |
| 210 | +- MD036: Emphasis used instead of heading (multiple lines) |
| 211 | + |
| 212 | +**Actions:** |
| 213 | + |
| 214 | +1. Convert emphasized text to proper headings |
| 215 | +2. Split long lines |
| 216 | + |
| 217 | +### tools/wt-worktree/notes.md |
| 218 | + |
| 219 | +**Issues:** |
| 220 | + |
| 221 | +- MD013: Multiple line length violations |
| 222 | +- MD040: Missing code block language (line 31) |
| 223 | + |
| 224 | +**Actions:** |
| 225 | + |
| 226 | +1. Split long lines |
| 227 | +2. Add ` ```bash ` to code block |
| 228 | + |
| 229 | +--- |
| 230 | + |
| 231 | +## 4. Configuration Options |
| 232 | + |
| 233 | +### Option 1: Fix all issues (Recommended for production) |
| 234 | + |
| 235 | +Work through each section above systematically. |
| 236 | + |
| 237 | +### Option 2: Relax some rules |
| 238 | + |
| 239 | +Add to `pyproject.toml`: |
| 240 | + |
| 241 | +```toml |
| 242 | +[tool.ruff.lint] |
| 243 | +ignore = [ |
| 244 | + # ... existing ignores ... |
| 245 | + "PTH123", # Allow open() instead of Path.open() |
| 246 | + "PLR0912", # Allow complex branches |
| 247 | + "PLR0915", # Allow many statements |
| 248 | +] |
| 249 | +``` |
| 250 | + |
| 251 | +Create `.markdownlint.json`: |
| 252 | + |
| 253 | +```json |
| 254 | +{ |
| 255 | + "MD013": { "line_length": 120 }, |
| 256 | + "MD033": false, |
| 257 | + "MD024": false, |
| 258 | + "MD036": false |
| 259 | +} |
| 260 | +``` |
| 261 | + |
| 262 | +### Option 3: Disable specific hooks temporarily |
| 263 | + |
| 264 | +Comment out hooks in `.pre-commit-config.yaml` while fixing issues incrementally. |
| 265 | + |
| 266 | +--- |
| 267 | + |
| 268 | +## Recommended Fix Order |
| 269 | + |
| 270 | +1. **Start with auto-fixable issues**: |
| 271 | + |
| 272 | + ```bash |
| 273 | + prek run ruff --all-files |
| 274 | + prek run ruff-format --all-files |
| 275 | + ``` |
| 276 | + |
| 277 | +2. **Fix manual ruff issues** (unused variables, pathlib migration) |
| 278 | + - Estimated effort: 2-3 hours |
| 279 | + - Files: ~30 Python files |
| 280 | + |
| 281 | +3. **Fix markdown issues** |
| 282 | + - Estimated effort: 1-2 hours |
| 283 | + - Or relax rules with `.markdownlint.json` |
| 284 | + |
| 285 | +4. **Fix type checking issues** (most complex) |
| 286 | + - Estimated effort: 3-4 hours |
| 287 | + - Or temporarily disable pyright hook |
| 288 | + |
| 289 | +5. **Re-run all hooks**: |
| 290 | + |
| 291 | + ```bash |
| 292 | + prek run --all-files |
| 293 | + ``` |
| 294 | + |
| 295 | +--- |
| 296 | + |
| 297 | +## Quick Win: Minimal Changes to Pass |
| 298 | + |
| 299 | +If you want to pass pre-commit quickly without major refactoring: |
| 300 | + |
| 301 | +1. Add to `pyproject.toml` ignores: |
| 302 | + |
| 303 | + ```toml |
| 304 | + "PTH123", "PTH118", "PTH119", # Allow os.path and open() |
| 305 | + "PLR0912", "PLR0915", # Allow complex functions |
| 306 | + "E741", # Allow short variable names |
| 307 | + "ARG001", # Allow unused arguments |
| 308 | + "C401", # Allow generators |
| 309 | + ``` |
| 310 | + |
| 311 | +2. Create `.markdownlint.json` with relaxed rules |
| 312 | + |
| 313 | +3. Temporarily comment out pyright hook in `.pre-commit-config.yaml` |
| 314 | + |
| 315 | +This will leave only critical issues (like unused variables and line length) to fix manually. |
0 commit comments