Skip to content

Modernise build tooling and configuration#79

Merged
j4mie merged 4 commits into
masterfrom
modernise-build-tooling
Feb 26, 2026
Merged

Modernise build tooling and configuration#79
j4mie merged 4 commits into
masterfrom
modernise-build-tooling

Conversation

@MarkBird

Copy link
Copy Markdown

Transition the project from the old-school setup.py style to the more modern pyproject.toml approach.

@dabapps-robot dabapps-robot self-requested a review February 26, 2026 14:30

@dabapps-robot dabapps-robot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice modernisation overall 👍 — this is a clean move from setup.py to pyproject.toml, and the CI publishing flow is much more current (using build + PyPI trusted publishing).

I think this is close, with a couple of things worth confirming before merge:

  1. requires-python changed to >=3.10

    • This is a significant support-policy change compared to the previous packaging setup (which built on 3.8).
    • If intentional, great — but it would be good to call it out explicitly in release notes/changelog.
    • If not intentional, you may want to keep this aligned with the project’s actual supported Python versions.
  2. GitHub Action now uses trusted publishing (pypa/gh-action-pypi-publish)

    • Good upgrade, but this depends on PyPI trusted publisher configuration for the repo/environment.
    • Worth double-checking that this is already configured, otherwise publish jobs will fail at release time.
  3. Package data behaviour changed

    • setup.py previously had custom get_package_data(...); the new config only declares package discovery.
    • If there are any non-Python files under log_request_id that should be included in distributions, confirm they’re still being shipped with the new build config.

If those points are intentional/covered, this looks good to merge.

@dabapps-robot dabapps-robot self-requested a review February 26, 2026 14:35

@dabapps-robot dabapps-robot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for the update — the migration itself still looks clean.

I don’t see anything new in the diff that resolves the three non-code confirmations from the previous review, so before merge I’d still like explicit confirmation on:

  • requires-python now being >=3.10 (intentional support-policy change, and documented accordingly).
  • PyPI trusted publisher setup being configured for this repo (since the workflow now depends on OIDC publish).
  • Whether any non-Python files previously included via get_package_data(...) are still included in built artifacts.

Once those are confirmed, this looks good to go.

@j4mie j4mie merged commit 7d5b615 into master Feb 26, 2026
11 checks passed
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