Skip to content

Closes #797, Closes #798: Adopt pyproject.toml and an explicit Ruff configuration - #803

Merged
pheus merged 7 commits into
mainfrom
798-ruff
Aug 27, 2026
Merged

Closes #797, Closes #798: Adopt pyproject.toml and an explicit Ruff configuration#803
pheus merged 7 commits into
mainfrom
798-ruff

Conversation

@arthanson

@arthanson arthanson commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Fixes: #798 #797

Adds explicit ruff configuration along with pyproject.toml settings file to set those params. Also fixes ruff line length errors. Updated python versions to match NetBox.

@arthanson arthanson changed the title 798 ruff 798 Add explicit Ruff configuration Aug 27, 2026
@arthanson

Copy link
Copy Markdown
Collaborator Author

@claude /review Only report on issues of potential concern; we don't need commentary on anything that doesn't require further consideration.

@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown

Claude finished @arthanson's task in 2m 34s —— View job


PR Review: #803 — Add explicit Ruff configuration

  • Gather diff and examine changed files
  • Review pyproject.toml configuration
  • Review code changes
  • Post findings

Two issues worth discussing before merging:


1. requires-python = ">=3.12" is a breaking change with no documentation

pyproject.toml:16

The old setup.py had no python_requires constraint — the package installed on any Python version. Setting >=3.12 will cause pip install pynetbox to silently fail (or refuse to install) for users on Python 3.10 or 3.11. While the CI matrix and current NetBox target 3.12+, there's a real installed base of users on older Pythons who would silently get no upgrade.

This warrants either:

  • A major version bump (v8.0.0) so pip's version solver can exclude the new release for older Pythons, or
  • A clear CHANGELOG/README entry so users are warned before upgrading.

2. preview = true in ruff config is semantically at odds with the stated goal

pyproject.toml:79

The comment explains that preview = true caused CI to go red on unmodified code between ruff releases, and the fix is to pin the effective rule set explicitly. But preview = true is still set — it just means the scope of instability is now limited to the selected namespaces (E1, E2, RET, UP, etc.) rather than all rules.

New preview rules can still be added to those namespaces in a future ruff point release and break CI on unmodified code — which is exactly the problem the comment describes. The only fully stable configurations are:

  • Pin ruff (already done) and drop preview = true, or
  • Keep preview = true and accept that bumping the ruff pin requires a lint-clean check.

The current setup (pinned ruff + preview = true + explicit selects) works fine today but leaves a trap for whoever next bumps the ruff pin in requirements-dev.txt and .pre-commit-config.yaml.


Everything else (import-order normalization, else-after-return removals, %.format() migration, setup.pypyproject.toml migration, doc rewraps) looks correct and clean.

@arthanson

Copy link
Copy Markdown
Collaborator Author

Per claude review: 1. it is more correct to pin the version number. 2. is just the comments - pyproject.toml is from netbox.

@arthanson
arthanson marked this pull request as ready for review August 27, 2026 16:18
@arthanson
arthanson requested a review from pheus August 27, 2026 16:18

@pheus pheus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clean-up, Arthur. This is in much better shape now.

I’m requesting changes because a few parts of the migration are still incomplete.

Comment thread pyproject.toml Outdated
Comment thread pyproject.toml Outdated
Comment thread pyproject.toml Outdated
Comment thread .pre-commit-config.yaml Outdated
Comment thread docs/installation.md Outdated
Comment thread AGENTS.md Outdated
@pheus pheus changed the title 798 Add explicit Ruff configuration Closes #797, Closes #798: Adopt pyproject.toml and an explicit Ruff configuration Aug 27, 2026

@pheus pheus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the follow-up work!

@pheus
pheus merged commit 54e71e2 into main Aug 27, 2026
10 checks passed
@pheus
pheus deleted the 798-ruff branch August 27, 2026 19:22
@pheus pheus linked an issue Aug 27, 2026 that may be closed by this pull request
@arthanson
arthanson requested a review from pheus August 27, 2026 19:55
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.

Adopt an explicit Ruff configuration Adopt pyproject.toml

2 participants