Convert python project configuration to pyproject.toml.#115
Convert python project configuration to pyproject.toml.#115roberthdevries wants to merge 1 commit intowolfSSL:masterfrom
Conversation
Modern python projects standardize on pyproject.toml to reduce the number of configuration files required for all tools.
|
Has requires-python = ">=3.10", current setup.py advertises 3.6-3.9 |
JeremiahM37
left a comment
JeremiahM37
left a comment
There was a problem hiding this comment.
Skoll Code Review
Scan type: review
Overall recommendation: REQUEST_CHANGES
Findings: 9 total — 2 posted, 7 skipped
2 finding(s) posted as inline comments (see file-level comments below)
Posted findings
- [High] Ruff
[lint]and[format]tables are at the top level instead of under[tool.ruff]—pyproject.toml:94-133 - [Medium]
requires-python = ">=3.10"silently drops Python 3.6–3.9 support —pyproject.toml:5
Skipped findings
- [High]
cffimissing from[build-system].requires— PEP 517 builds will fail - [Medium]
build-backend = setuptools.build_meta:__legacy__contradicts the PR's goal - [Medium]
dynamic = ["version"]declared without[tool.setuptools.dynamic]configuration - [Low]
Classifiers list only Python 3.10 even though 3.11/3.12/3.13 are allowed - [Medium]
[build-system].requires = ["setuptools"]not pinned to a version supporting SPDXlicense/license-files - [Low]
Tox config inpyproject.tomlmay not be picked up becausepackageis a string instead of a wheel-package env reference - [Low]
setup_requiresleft insetup.py— dead code under PEP 517
Review generated by Skoll
| [lint] | ||
| # Enable Pyflakes (`F`) and a subset of the pycodestyle (`E`) codes by default. | ||
| # Unlike Flake8, Ruff doesn't enable pycodestyle warnings (`W`) or | ||
| # McCabe complexity (`C901`) by default. | ||
| select = ["E4", "E7", "E9", "F", "B", "UP"] | ||
| ignore = ["UP031", "UP025", "UP032"] | ||
|
|
||
| # Allow fix for all enabled rules (when `--fix`) is provided. | ||
| fixable = ["ALL"] | ||
| unfixable = [] | ||
|
|
||
| # Allow unused variables when underscore-prefixed. | ||
| dummy-variable-rgx = "^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$" | ||
|
|
||
| [format] | ||
| # Like Black, use double quotes for strings. | ||
| quote-style = "double" | ||
|
|
||
| # Like Black, indent with spaces, rather than tabs. | ||
| indent-style = "space" | ||
|
|
||
| # Like Black, respect magic trailing commas. | ||
| skip-magic-trailing-comma = false | ||
|
|
||
| # Like Black, automatically detect the appropriate line ending. | ||
| line-ending = "auto" | ||
|
|
||
| # Enable auto-formatting of code examples in docstrings. Markdown, | ||
| # reStructuredText code/literal blocks and doctests are all supported. | ||
| # | ||
| # This is currently disabled by default, but it is planned for this | ||
| # to be opt-out in the future. | ||
| docstring-code-format = false | ||
|
|
||
| # Set the line length limit used when formatting code snippets in | ||
| # docstrings. | ||
| # | ||
| # This only has an effect when the `docstring-code-format` setting is | ||
| # enabled. | ||
| docstring-code-line-length = "dynamic" |
There was a problem hiding this comment.
🔴 [High] Ruff [lint] and [format] tables are at the top level instead of under [tool.ruff]
Ruff reads its configuration from [tool.ruff], [tool.ruff.lint], and [tool.ruff.format] when configured via pyproject.toml. The diff places [lint] and [format] as top-level TOML tables, so ruff will silently ignore them — the select/ignore rules, quote-style, indent-style, etc. will not take effect. Only the [tool.ruff] section above (exclude/line-length/target-version) will be applied. This silently regresses lint/format behavior compared to a working ruff.toml or correctly-namespaced config.
[lint]
# Enable Pyflakes (`F`) and a subset of the pycodestyle (`E`) codes by default.
...
select = ["E4", "E7", "E9", "F", "B", "UP"]
ignore = ["UP031", "UP025", "UP032"]
...
[format]
quote-style = "double"Recommendation: Rename the [lint] and [format] tables to [tool.ruff.lint] and [tool.ruff.format] so ruff actually picks up the configuration.
| name = "wolfcrypt" | ||
| description = "Python module that encapsulates wolfSSL's crypto engine API." | ||
| readme = "README.rst" | ||
| requires-python = ">=3.10" |
There was a problem hiding this comment.
🟠 [Medium] requires-python = ">=3.10" silently drops Python 3.6–3.9 support
The previous setup.py advertised support for Python 3.6, 3.7, 3.8, and 3.9 via classifiers (with no explicit python_requires). The new metadata bumps the floor to Python 3.10, which is a backward-incompatible change for downstream users still on 3.8/3.9 (still in widespread enterprise/distro use as of 2026). The PR description (Modern python projects standardize on pyproject.toml…) only mentions configuration consolidation, not a Python-version bump. If this is intentional it should be called out in the PR description and/or CHANGELOG; if not, restore the lower floor (e.g. >=3.8).
requires-python = ">=3.10"Recommendation: Confirm the Python 3.10 floor is intentional and document the deprecation in the changelog; otherwise lower the floor back to the previously-supported version.
3 participants
Modern python projects standardize on pyproject.toml to reduce the number of configuration files required for all tools.