Skip to content

Convert python project configuration to pyproject.toml.#115

Open
roberthdevries wants to merge 2 commits intowolfSSL:masterfrom
roberthdevries:add-pyproject-toml
Open

Convert python project configuration to pyproject.toml.#115
roberthdevries wants to merge 2 commits intowolfSSL:masterfrom
roberthdevries:add-pyproject-toml

Conversation

@roberthdevries
Copy link
Copy Markdown
Contributor

Modern python projects standardize on pyproject.toml to reduce the number of configuration files required for all tools.

Modern python projects standardize on pyproject.toml to reduce
the number of configuration files required for all tools.
@JeremiahM37
Copy link
Copy Markdown
Contributor

Has requires-python = ">=3.10", current setup.py advertises 3.6-3.9

Copy link
Copy Markdown
Contributor

@JeremiahM37 JeremiahM37 left a comment

Choose a reason for hiding this comment

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

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 supportpyproject.toml:5

Skipped findings

  • [High] cffi missing 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 SPDX license / license-files
  • [Low] Tox config in pyproject.toml may not be picked up because package is a string instead of a wheel-package env reference
  • [Low] setup_requires left in setup.py — dead code under PEP 517

Review generated by Skoll

Comment thread pyproject.toml Outdated
Comment thread pyproject.toml
name = "wolfcrypt"
description = "Python module that encapsulates wolfSSL's crypto engine API."
readme = "README.rst"
requires-python = ">=3.10"
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.

🟠 [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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is intentional as older Python version are end-of-life. Ref: https://devguide.python.org/versions/

A note will be added to the Release Notes.

I tested with Python versions 3.10-3.15 that the software passes the unit tests which means that the statement above is actually true.

There is already one other patches (not merged) that adds something to the ChangeLog. Adding something now will create a merge conflict, as is true for some other patches as well. It would help a lot if these patches were merged on short notice, to help preventing this in the near future.

Also add entry in ChangeLog mentioning that older Python versions
are no longer supported.
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.

3 participants