Skip to content

Lint and format the files excluded from ruff #394

Open

Description

@bjester

❌ This issue is not open for contribution. Visit Contributing guidelines to learn about the contributing process and how to find suitable issues.

Overview

Evaluate the files that ruff excludes in pyproject.toml, and bring them under linting and formatting where that is practical.

Background & Motivation

[tool.ruff].exclude in pyproject.toml lists these paths:

"morango/*/migrations/*",
"docs",
"morango/sync/operations.py",
"morango/sync/backends/postgres.py",
"morango/sync/backends/sqlite.py",
"tests/testapp/tests/test_crypto.py",

The list is older than ruff. It was the flake8 exclude list in setup.cfg, added in 805967a (2019, "Refactor project structure"), with no recorded reason. The uv migration (665c52f) and the ruff migration (292f58e, #311) carried it over unchanged.

With ruff, the exclude covers more than it did before:

  • Before ruff, only flake8 skipped these files. reorder-python-imports still sorted their imports, and the project had no formatter.
  • With ruff, the same list skips linting, import sorting and formatting. The ruff-pre-commit hooks pass --force-exclude, so the hooks skip these files even when a commit changes them.

This caused a real gotcha in #392. Removing the legacy deserialization code from morango/sync/operations.py left 19 unused imports, and prek reported nothing. They were found only by running ruff on the file by hand.

User Story / Use Case

As a Morango developer,
I want every Python file that I change to be checked by the same lint and format hooks,
So that unused imports, unsorted imports and style drift are caught at commit time, not in review.

Description & Expected Outcomes

First evaluate, then decide. For each excluded path, find out what removing the exclude means, and whether it should be removed.

A first measurement with ruff 0.15.16 (the pinned hook version) on release-v0.9.x (4eacad2), with the exclude list emptied:

Path Lint findings Formatter changes
morango/sync/operations.py 1 I001; 1 C901 (_deserialize_from_store, which #392 removes) ~163 changed lines
morango/sync/backends/postgres.py 1 I001; 1 F522 (unused rmc= argument to a SQL .format()) ~28 changed lines
morango/sync/backends/sqlite.py 1 I001; 1 F522 (same) ~16 changed lines
tests/testapp/tests/test_crypto.py none ~50 changed lines
docs (only docs/conf.py) none would be reformatted
morango/*/migrations/* n/a n/a

Notes on the measurement:

  • The migrations pattern matches nothing. Migrations live in morango/migrations/, not morango/*/migrations/. Ruff already lints and formats all 30 migration files, and they pass. Remove the entry, or fix the pattern if migrations should be excluded on purpose.
  • Formatter changes are mostly line joins. Most of the code was wrapped for a shorter line length than the current 100 characters. The rest is trailing commas and blank lines. The multi-line SQL inside string literals is not changed.
  • The F522 auto-fix changes code. It removes the unused keyword argument. Keep it out of any formatting-only commit.
  • The numbers change as other work lands. For example, on the Streaming deserialization part 3: sink #392 branch operations.py has no lint findings and about 100 formatter lines.

Value Add & Impact

  • Commit hooks give the same checks for every Python file.
  • Fewer review comments about style and unused code in the sync operations and backends. These are some of the most-changed files in the repository.
  • One less config surprise for new contributors.

Deliverables & Contracts

  • A short record (in the PR description is enough) of the decision for each excluded path: remove the exclude, keep it with a stated reason, or narrow it.
  • Each path that is no longer excluded passes prek run --all-files.
  • Any change made only by reformatting is in its own commit, with no other changes. That commit's full SHA is added to .git-blame-ignore-revs, as was done for 292f58e and cff1e87.
  • Lint fixes that change code (for example F522) are in separate commits from the reformatting.

Acceptance Criteria

  • Each entry in [tool.ruff].exclude is removed, or kept with a reason in a comment in pyproject.toml.
  • The morango/*/migrations/* entry is removed or corrected, so no entry matches nothing.
  • prek run --all-files passes.
  • git blame with blame.ignoreRevsFile set shows the pre-reformat authors for the reformatted files.
  • make test passes on SQLite and PostgreSQL.

Technical Pointers & Architecture

  • Target Components / Context: pyproject.toml ([tool.ruff]), .pre-commit-config.yaml, .git-blame-ignore-revs, and the excluded files listed above.
  • Related Patterns: 292f58e ("replace flake8/isort with ruff") and cff1e87 ("Reformat imports to new single-line settings") are formatting commits already in .git-blame-ignore-revs.
  • Data Model & Schema Considerations: None.
  • Resilience & Failure Modes: The backends and operations.py build raw SQL. Make sure no change touches SQL string contents. Run the full suite on both databases.

Notes & Tradeoffs

  • Dependencies: Best done after Streaming deserialization part 3: sink #392 merges, because Streaming deserialization part 3: sink #392 removes a large part of operations.py. Reformatting first would conflict with it.
  • Testing Requirements: Run the full suite on SQLite and PostgreSQL. The backends hold SQL for each database.
  • Tradeoffs & Alternatives Considered:
    • Keeping the excludes avoids a reformatting diff, but leaves the hooks silent on these files.
    • Lint-only inclusion is possible, through [tool.ruff.lint].exclude and [tool.ruff.format].exclude. This keeps the formatting stable while still catching unused imports.
    • Reformatting causes merge conflicts with open branches that touch these files. .git-blame-ignore-revs keeps git blame useful.
  • Compatibility & Migrations: No runtime behavior change is expected.

Metadata

Complexity: Low
Target Branch: release-v0.9.x
Labels: TAG: tech update / debt, TAG: dev experience

AI Usage

Drafted with Claude Code, after we found the gotcha while working on #392. Claude traced the history of the exclude list in git and measured the effect of removing it with the pinned ruff version (0.15.16) on release-v0.9.x. It also confirmed that the migrations pattern matches no files. I asked for the issue and set its scope: evaluate before deciding, and record any reformatting in .git-blame-ignore-revs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions