Switch from setup.py to setup.cfg and dynamic version based off GIT tags#325
Switch from setup.py to setup.cfg and dynamic version based off GIT tags#325tbooth wants to merge 3 commits intobw2:masterfrom
Conversation
…ags. I tested this extensively on my own fork but I've had to make things compatible with the other things in upstream so there may be snags to work out.
There was a problem hiding this comment.
Pull Request Overview
This PR replaces the custom setup.py with declarative configuration in setup.cfg and uses Git tags for dynamic versioning, while updating CI flows and helper scripts accordingly.
- Removed
setup.pyand centralized package metadata insetup.cfg+pyproject.toml - Adjusted tests (
tox.ini) and GitHub Actions to use modern build tooling and tag‐based releases - Updated
apidocs.shandCONTRIBUTING.rstto reflect new versioning commands
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tox.ini | Switched from python setup.py test to python -m unittest discover |
| setup.py | Fully removed legacy setup script |
| setup.cfg | Declared package metadata and dynamic version via git |
| pyproject.toml | Added PEP 517 build-system and versioning backend |
| apidocs.sh | Changed version lookup and improved cd invocation |
| CONTRIBUTING.rst | Updated release instructions to tag v<version> |
| .github/workflows/test.yml | Updated install/build commands and release trigger for refs/tags/v |
| .github/workflows/apidocs.yml | Enabled full repo fetch (depth and tags) for docs build |
Comments suppressed due to low confidence (3)
.github/workflows/apidocs.yml:15
- The
with:block is indented as its own step instead of under theactions/checkoutstep. Merge it under theuses: actions/checkout@...entry to correctly configurefetch-depthandfetch-tags.
- with:
setup.cfg:24
- [nitpick] Moving
PyYAMLfrom an optional extra to a mandatory install requirement changes the packageʼs minimal dependencies. If YAML support should remain optional, restore it underextras_requireinstead.
PyYAML
apidocs.sh:12
- The script tries to invoke
setuptools-git-versioningdirectly, but that is not a standalone CLI. Consider usingpython3 -m setuptools_git_versioningor reading the version viagit describe --tags.
project_version="$(setuptools-git-versioning)"
kingbuzzman
left a comment
kingbuzzman
left a comment
There was a problem hiding this comment.
This is my opinion, but look at this
https://github.com/kingbuzzman/django-squash/blob/master/pyproject.toml#L7
https://github.com/kingbuzzman/django-squash/blob/master/.github/workflows/release.yml
you keep the version static in the project, and ci then figures out if there is a tag created for it, if there isn't, it knows it needs to create one, and push it, otherwise it does nothing.
in terms of security, this release workflow has two features, 1 it's using a variable flagged as "deploy" workflow, which means in order to get the value of the pypi you need to be authorized.
if the project ever needs to know what version it is on
https://github.com/kingbuzzman/django-squash/blob/master/django_squash/__init__.py
again, this is my opinion, but i'm not convinced git is a good way to do store the version.
| steps: | ||
| - uses: actions/checkout@master | ||
| - with: | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
This will cause slow downs in the future, rather this stay as it was
There was a problem hiding this comment.
This change is necessary for using setuptools-git-versioning. If we decide not to use setuptools-git-versioning I can revert it.
| - name: Build | ||
| run: | | ||
| python setup.py --quiet build check sdist bdist_wheel | ||
| rm -rf dist |
There was a problem hiding this comment.
This line will do nothing, ci will always start a new session with nothing cached. The edge case that this will fix, is more confusing than anything. i'd would remove this
There was a problem hiding this comment.
I don't see the problem
There was a problem hiding this comment.
Its as effective as as adding: # this is a comment instead of the rm .. 😄
| requires = ["setuptools>=41", "wheel", "setuptools-git-versioning"] | ||
| build-backend = "setuptools.build_meta" | ||
|
|
||
| [tool.setuptools-git-versioning] |
There was a problem hiding this comment.
not a fan of this, i'd rather the version be defined, static, in the pyproject.toml/setup.cfg
There was a problem hiding this comment.
I have tested and submitted this based on the discussions in the teleconference but I'll switch back to a static version if that's the decision. My concern is that currently the GIT tags and the version may conflict whereas this gives us a single source of truth.
There was a problem hiding this comment.
My concern is that currently the GIT tags and the version may conflict whereas this gives us a single source of truth.
Very valid concern, im just not convinced git is the place to put it.
|
|
||
| commands = python setup.py test | ||
| # python -m unittest discover | ||
| commands = python -m unittest discover |
There was a problem hiding this comment.
this has nothing to do with the rest of the pr, please split this in another pr... plus we don't use tox...
There was a problem hiding this comment.
This PR removes setup.py so it is correct to remove calls to setup.py. If you want to remove tox entirely then I would suggest this should be done in a separate PR.
2 participants
I tested all this on my own fork but I've had to merge the changes back into the master branch and make things compatible with the other things in upstream so there may be snags to work out.
If automated publishing direct to PyPi is problematic then I have an alternative suggestion which I also tested. That is to make a draft release in GitHub containing the
dist/files as assets, and then this can be uploaded to PyPi after a quick sanity check.