Skip to content

pyproject(ruff): Add ICN, PT, N + exception Error-suffix rename#420

Open
tony wants to merge 5 commits into
masterfrom
ruff-more-rules
Open

pyproject(ruff): Add ICN, PT, N + exception Error-suffix rename#420
tony wants to merge 5 commits into
masterfrom
ruff-more-rules

Conversation

@tony

@tony tony commented Sep 24, 2023

Copy link
Copy Markdown
Member

Adopts three additional ruff rule sets, regenerated fresh against current master.

What changed

  • ICN (flake8-import-conventions) — enabled; zero violations.
  • PT (flake8-pytest-style) — enabled; fixtures converted to yield teardown, compound assertions split, pytest.raises() blocks narrowed, value-less fixture switched to usefixtures.
  • N (pep8-naming) — enabled; module_funcName lowercased, test sentinel suffixed.

Breaking change

Two public exceptions were renamed with an Error suffix (N818), no aliases kept:

  • VCSPullExceptionVCSPullError
  • MultipleConfigWarningMultipleConfigError (it was always raised as a hard error)

Update except clauses accordingly. See the Breaking changes entry in CHANGES.

Note

D (pydocstyle) is already enabled on master, so it is not part of this PR; the original branch's D addition and its tests/* = ["D"] ignore were dropped (the ignore would have regressed test docstring coverage).

@tony tony force-pushed the master branch 4 times, most recently from 206715e to 0f25cdd Compare November 25, 2024 23:02
@tony tony force-pushed the ruff-more-rules branch from f5a4d0f to f040b78 Compare July 5, 2026 13:51
@tony tony changed the title pyproject(ruff): Add ICN, N, D, PT pyproject(ruff): Add ICN, PT, N + exception Error-suffix rename Jul 5, 2026
@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.95238% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.33%. Comparing base (e6a62d9) to head (e8a1193).

Files with missing lines Patch % Lines
conftest.py 60.00% 2 Missing ⚠️
src/vcspull/config.py 80.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #420      +/-   ##
==========================================
+ Coverage   83.30%   83.33%   +0.02%     
==========================================
  Files          32       32              
  Lines        4492     4488       -4     
  Branches      903      903              
==========================================
- Hits         3742     3740       -2     
+ Misses        499      497       -2     
  Partials      251      251              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

tony added 5 commits July 5, 2026 09:57
why: The codebase already follows conventional import aliasing; enabling
ICN locks that convention in and flags any future drift at lint time.

what:
- Add "ICN" to [tool.ruff.lint].select (zero existing violations)
why: PT catches pytest anti-patterns that make fixtures leak and failures
hard to localize -- finalizer-based teardown that skips on setup error,
compound assertions that hide which half failed, and wide raises() blocks
that can mask where the exception actually came from.

what:
- Add "PT" to [tool.ruff.lint].select
- conftest: convert config_path/repos_path finalizers to yield teardown
- Split compound assertions in test_sync_sigint/test_sync_watchdog
- Narrow pytest.raises() blocks to the raising call in test_config_file,
  test_worktree; hoist setup/casts out of the block
- Use @pytest.mark.usefixtures for the value-less _fake_sigint_escalation
why: Two public exceptions did not follow the Error-suffix naming
convention (pep8-naming N818): the base VCSPullException, and
MultipleConfigWarning -- which despite its name is always raised as a
hard error, never a warning. Aligning both lets the N rule be enabled
without per-symbol suppressions.

BREAKING: `except vcspull.exc.VCSPullException` and `except
vcspull.exc.MultipleConfigWarning` no longer match; catch VCSPullError /
MultipleConfigError instead. No aliases are kept.

what:
- Rename VCSPullException -> VCSPullError and MultipleConfigWarning ->
  MultipleConfigError in exc.py
- Update all internal references (config, cli/sync, cli/import_cmd, tests)
  and the exc API doc reference
why: The exception renames are breaking changes for downstream callers
catching them; the changelog must flag both with a migration path before
the next release.

what:
- Add a Breaking changes entry covering VCSPullError and
  MultipleConfigError with before/after except-clause guidance
why: N surfaces identifiers that drift from Python naming norms --
camelCase locals and exception classes without an Error suffix -- which
hurt grep-ability and readability. Enabling it after the exception
renames keeps the rule clean with no suppressions.

what:
- Add "N" to [tool.ruff.lint].select
- Lowercase the module_funcName local in log.py (N806)
- Suffix the test-local _Boom sentinel as _BoomError (N818)
@tony tony force-pushed the ruff-more-rules branch from f040b78 to e8a1193 Compare July 5, 2026 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant